[BK PATCH] Add TPM driver support for 2.6.11

[BK PATCH] Add TPM driver support for 2.6.11

[Greg KH]

Hi,

Here are a few changesets that add support for TPM drivers.  These
patches have all been in the -mm releases for a while now.

Please pull from:  bk://kernel.bkbits.net/gregkh/linux/2.6.11/tpm

Individual patches will follow, sent to the linux-kernel list.

thanks,

greg k-h

 drivers/char/Kconfig         |    2 
 drivers/char/Makefile        |    2 
 drivers/char/tpm/Kconfig     |   39 ++
 drivers/char/tpm/Makefile    |    7 
 drivers/char/tpm/tpm.c       |  715 ++++++++++++++++++++++++++++++++++++++++++-
 drivers/char/tpm/tpm.h       |   92 +++++
 drivers/char/tpm/tpm_atmel.c |  218 +++++++++++++
 drivers/char/tpm/tpm_nsc.c   |  375 ++++++++++++++++++++++
 include/linux/pci_ids.h      |    1 
 9 files changed, 1439 insertions(+), 12 deletions(-)
-----


<kjhall:us.ibm.com>:
  o tpm: fix cause of SMP stack traces
  o Add TPM hardware enablement driver

Andrew Morton:
  o tpm-build-fix
  o tpm_atmel build fix
  o tpm_msc-build-fix

[PATCH] Add TPM hardware enablement driver

[Greg KH]

ChangeSet 1.2035, 2005/03/09 10:12:19-08:00, kjhall@us.ibm.com

[PATCH] Add TPM hardware enablement driver

This patch is a device driver to enable new hardware.  The new hardware is
the TPM chip as described by specifications at
<http://www.trustedcomputinggroup.org>.  The TPM chip will enable you to
use hardware to securely store and protect your keys and personal data.
To use the chip according to the specification, you will need the Trusted
Software Stack (TSS) of which an implementation for Linux is available at:
<http://sourceforge.net/projects/trousers>.


Signed-off-by: Leendert van Doorn <leendert@watson.ibm.com>
Signed-off-by: Reiner Sailer <sailer@watson.ibm.com>
Signed-off-by: Dave Safford <safford@watson.ibm.com>
Signed-off-by: Kylene Hall <kjhall@us.ibm.com>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>


 drivers/char/Kconfig         |    2 
 drivers/char/Makefile        |    2 
 drivers/char/tpm/Kconfig     |   39 ++
 drivers/char/tpm/Makefile    |    7 
 drivers/char/tpm/tpm.c       |  697 +++++++++++++++++++++++++++++++++++++++++++
 drivers/char/tpm/tpm.h       |   92 +++++
 drivers/char/tpm/tpm_atmel.c |  216 +++++++++++++
 drivers/char/tpm/tpm_nsc.c   |  372 ++++++++++++++++++++++
 include/linux/pci_ids.h      |    1 
 9 files changed, 1427 insertions(+), 1 deletion(-)


diff -Nru a/drivers/char/Kconfig b/drivers/char/Kconfig
--- a/drivers/char/Kconfig	2005-03-09 16:40:26 -08:00
+++ b/drivers/char/Kconfig	2005-03-09 16:40:26 -08:00
@@ -995,5 +995,7 @@
 	  The mmtimer device allows direct userspace access to the
 	  Altix system timer.
 
+source "drivers/char/tpm/Kconfig"
+
 endmenu
 
diff -Nru a/drivers/char/Makefile b/drivers/char/Makefile
--- a/drivers/char/Makefile	2005-03-09 16:40:26 -08:00
+++ b/drivers/char/Makefile	2005-03-09 16:40:26 -08:00
@@ -89,7 +89,7 @@
 obj-$(CONFIG_IPMI_HANDLER) += ipmi/
 
 obj-$(CONFIG_HANGCHECK_TIMER) += hangcheck-timer.o
-
+obj-$(CONFIG_TCG_TPM) += tpm/
 # Files generated that shall be removed upon make clean
 clean-files := consolemap_deftbl.c defkeymap.c qtronixmap.c
 
diff -Nru a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
--- /dev/null	Wed Dec 31 16:00:00 196900
+++ b/drivers/char/tpm/Kconfig	2005-03-09 16:40:26 -08:00
@@ -0,0 +1,39 @@
+#
+# TPM device configuration
+#
+
+menu "TPM devices"
+
+config TCG_TPM
+	tristate "TPM Hardware Support"
+	depends on EXPERIMENTAL
+	---help---
+	  If you have a TPM security chip in your system, which
+	  implements the Trusted Computing Group's specification,
+	  say Yes and it will be accessible from within Linux.  For
+	  more information see <http://www.trustedcomputinggroup.org>. 
+	  An implementation of the Trusted Software Stack (TSS), the 
+	  userspace enablement piece of the specification, can be 
+	  obtained at: <http://sourceforge.net/projects/trousers>.  To 
+	  compile this driver as a module, choose M here; the module 
+	  will be called tpm. If unsure, say N.
+
+config TCG_NSC
+	tristate "National Semiconductor TPM Interface"
+	depends on TCG_TPM
+	---help---
+	  If you have a TPM security chip from National Semicondutor 
+	  say Yes and it will be accessible from within Linux.  To 
+	  compile this driver as a module, choose M here; the module 
+	  will be called tpm_nsc.
+
+config TCG_ATMEL
+	tristate "Atmel TPM Interface"
+	depends on TCG_TPM
+	---help---
+	  If you have a TPM security chip from Atmel say Yes and it 
+	  will be accessible from within Linux.  To compile this driver 
+	  as a module, choose M here; the module will be called tpm_atmel.
+
+endmenu
+
diff -Nru a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
--- /dev/null	Wed Dec 31 16:00:00 196900
+++ b/drivers/char/tpm/Makefile	2005-03-09 16:40:26 -08:00
@@ -0,0 +1,7 @@
+#
+# Makefile for the kernel tpm device drivers.
+#
+obj-$(CONFIG_TCG_TPM) += tpm.o
+obj-$(CONFIG_TCG_NSC) += tpm_nsc.o
+obj-$(CONFIG_TCG_ATMEL) += tpm_atmel.o
+
diff -Nru a/drivers/char/tpm/tpm.c b/drivers/char/tpm/tpm.c
--- /dev/null	Wed Dec 31 16:00:00 196900
+++ b/drivers/char/tpm/tpm.c	2005-03-09 16:40:26 -08:00
@@ -0,0 +1,697 @@
+/*
+ * Copyright (C) 2004 IBM Corporation
+ *
+ * Authors:
+ * Leendert van Doorn <leendert@watson.ibm.com>
+ * Dave Safford <safford@watson.ibm.com>
+ * Reiner Sailer <sailer@watson.ibm.com>
+ * Kylene Hall <kjhall@us.ibm.com>
+ *
+ * Maintained by: <tpmdd_devel@lists.sourceforge.net>
+ *
+ * Device driver for TCG/TCPA TPM (trusted platform module).
+ * Specifications at www.trustedcomputinggroup.org	 
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation, version 2 of the
+ * License.
+ * 
+ * Note, the TPM chip is not interrupt driven (only polling)
+ * and can have very long timeouts (minutes!). Hence the unusual
+ * calls to schedule_timeout.
+ *
+ */
+
+#include <linux/sched.h>
+#include <linux/poll.h>
+#include <linux/spinlock.h>
+#include "tpm.h"
+
+#define	TPM_MINOR			224	/* officially assigned */
+
+#define	TPM_BUFSIZE			2048
+
+/* PCI configuration addresses */
+#define	PCI_GEN_PMCON_1			0xA0
+#define	PCI_GEN1_DEC			0xE4
+#define	PCI_LPC_EN			0xE6
+#define	PCI_GEN2_DEC			0xEC
+
+static LIST_HEAD(tpm_chip_list);
+static spinlock_t driver_lock = SPIN_LOCK_UNLOCKED;
+static int dev_mask[32];
+
+static void user_reader_timeout(unsigned long ptr)
+{
+	struct tpm_chip *chip = (struct tpm_chip *) ptr;
+
+	down(&chip->buffer_mutex);
+	atomic_set(&chip->data_pending, 0);
+	memset(chip->data_buffer, 0, TPM_BUFSIZE);
+	up(&chip->buffer_mutex);
+}
+
+void tpm_time_expired(unsigned long ptr)
+{
+	int *exp = (int *) ptr;
+	*exp = 1;
+}
+
+EXPORT_SYMBOL_GPL(tpm_time_expired);
+
+/*
+ * Initialize the LPC bus and enable the TPM ports
+ */
+int tpm_lpc_bus_init(struct pci_dev *pci_dev, u16 base)
+{
+	u32 lpcenable, tmp;
+	int is_lpcm = 0;
+
+	switch (pci_dev->vendor) {
+	case PCI_VENDOR_ID_INTEL:
+		switch (pci_dev->device) {
+		case PCI_DEVICE_ID_INTEL_82801CA_12:
+		case PCI_DEVICE_ID_INTEL_82801DB_12:
+			is_lpcm = 1;
+			break;
+		}
+		/* init ICH (enable LPC) */
+		pci_read_config_dword(pci_dev, PCI_GEN1_DEC, &lpcenable);
+		lpcenable |= 0x20000000;
+		pci_write_config_dword(pci_dev, PCI_GEN1_DEC, lpcenable);
+
+		if (is_lpcm) {
+			pci_read_config_dword(pci_dev, PCI_GEN1_DEC,
+					      &lpcenable);
+			if ((lpcenable & 0x20000000) == 0) {
+				dev_err(&pci_dev->dev,
+					"cannot enable LPC\n");
+				return -ENODEV;
+			}
+		}
+
+		/* initialize TPM registers */
+		pci_read_config_dword(pci_dev, PCI_GEN2_DEC, &tmp);
+
+		if (!is_lpcm)
+			tmp = (tmp & 0xFFFF0000) | (base & 0xFFF0);
+		else
+			tmp =
+			    (tmp & 0xFFFF0000) | (base & 0xFFF0) |
+			    0x00000001;
+
+		pci_write_config_dword(pci_dev, PCI_GEN2_DEC, tmp);
+
+		if (is_lpcm) {
+			pci_read_config_dword(pci_dev, PCI_GEN_PMCON_1,
+					      &tmp);
+			tmp |= 0x00000004;	/* enable CLKRUN */
+			pci_write_config_dword(pci_dev, PCI_GEN_PMCON_1,
+					       tmp);
+		}
+		tpm_write_index(0x0D, 0x55);	/* unlock 4F */
+		tpm_write_index(0x0A, 0x00);	/* int disable */
+		tpm_write_index(0x08, base);	/* base addr lo */
+		tpm_write_index(0x09, (base & 0xFF00) >> 8);	/* base addr hi */
+		tpm_write_index(0x0D, 0xAA);	/* lock 4F */
+		break;
+	case PCI_VENDOR_ID_AMD:
+		/* nothing yet */
+		break;
+	}
+
+	return 0;
+}
+
+EXPORT_SYMBOL_GPL(tpm_lpc_bus_init);
+
+/*
+ * Internal kernel interface to transmit TPM commands
+ */
+static ssize_t tpm_transmit(struct tpm_chip *chip, const char *buf,
+			    size_t bufsiz)
+{
+	ssize_t len;
+	u32 count;
+	__be32 *native_size;
+
+	native_size = (__force __be32 *) (buf + 2);
+	count = be32_to_cpu(*native_size);
+
+	if (count == 0)
+		return -ENODATA;
+	if (count > bufsiz) {
+		dev_err(&chip->pci_dev->dev,
+			"invalid count value %x %x \n", count, bufsiz);
+		return -E2BIG;
+	}
+
+	down(&chip->tpm_mutex);
+
+	if ((len = chip->vendor->send(chip, (u8 *) buf, count)) < 0) {
+		dev_err(&chip->pci_dev->dev,
+			"tpm_transmit: tpm_send: error %d\n", len);
+		return len;
+	}
+
+	down(&chip->timer_manipulation_mutex);
+	chip->time_expired = 0;
+	init_timer(&chip->device_timer);
+	chip->device_timer.function = tpm_time_expired;
+	chip->device_timer.expires = jiffies + 2 * 60 * HZ;
+	chip->device_timer.data = (unsigned long) &chip->time_expired;
+	add_timer(&chip->device_timer);
+	up(&chip->timer_manipulation_mutex);
+
+	do {
+		u8 status = inb(chip->vendor->base + 1);
+		if ((status & chip->vendor->req_complete_mask) ==
+		    chip->vendor->req_complete_val) {
+			down(&chip->timer_manipulation_mutex);
+			del_singleshot_timer_sync(&chip->device_timer);
+			up(&chip->timer_manipulation_mutex);
+			goto out_recv;
+		}
+		set_current_state(TASK_UNINTERRUPTIBLE);
+		schedule_timeout(TPM_TIMEOUT);
+		rmb();
+	} while (!chip->time_expired);
+
+
+	chip->vendor->cancel(chip);
+	dev_err(&chip->pci_dev->dev, "Time expired\n");
+	up(&chip->tpm_mutex);
+	return -EIO;
+
+out_recv:
+	len = chip->vendor->recv(chip, (u8 *) buf, bufsiz);
+	if (len < 0)
+		dev_err(&chip->pci_dev->dev,
+			"tpm_transmit: tpm_recv: error %d\n", len);
+	up(&chip->tpm_mutex);
+	return len;
+}
+
+#define TPM_DIGEST_SIZE 20
+#define CAP_PCR_RESULT_SIZE 18
+static u8 cap_pcr[] = {
+	0, 193,			/* TPM_TAG_RQU_COMMAND */
+	0, 0, 0, 22,		/* length */
+	0, 0, 0, 101,		/* TPM_ORD_GetCapability */
+	0, 0, 0, 5,
+	0, 0, 0, 4,
+	0, 0, 1, 1
+};
+
+#define READ_PCR_RESULT_SIZE 30
+static u8 pcrread[] = {
+	0, 193,			/* TPM_TAG_RQU_COMMAND */
+	0, 0, 0, 14,		/* length */
+	0, 0, 0, 21,		/* TPM_ORD_PcrRead */
+	0, 0, 0, 0		/* PCR index */
+};
+
+static ssize_t show_pcrs(struct device *dev, char *buf)
+{
+	u8 data[READ_PCR_RESULT_SIZE];
+	ssize_t len;
+	int i, j, index, num_pcrs;
+	char *str = buf;
+
+	struct tpm_chp *chip =
+	    pci_get_drvdata(container_of(dev, struct pci_dev, dev));
+	if (chip == NULL)
+		return -ENODEV;
+
+	memcpy(data, cap_pcr, sizeof(cap_pcr));
+	if ((len = tpm_transmit(chip, data, sizeof(data)))
+	    < CAP_PCR_RESULT_SIZE)
+		return len;
+
+	num_pcrs = be32_to_cpu(*((__force __be32 *) (data + 14)));
+
+	for (i = 0; i < num_pcrs; i++) {
+		memcpy(data, pcrread, sizeof(pcrread));
+		index = cpu_to_be32(i);
+		memcpy(data + 10, &index, 4);
+		if ((len = tpm_transmit(chip, data, sizeof(data)))
+		    < READ_PCR_RESULT_SIZE)
+			return len;
+		str += sprintf(str, "PCR-%02d: ", i);
+		for (j = 0; j < TPM_DIGEST_SIZE; j++)
+			str += sprintf(str, "%02X ", *(data + 10 + j));
+		str += sprintf(str, "\n");
+	}
+	return str - buf;
+}
+
+static DEVICE_ATTR(pcrs, S_IRUGO, show_pcrs, NULL);
+
+#define  READ_PUBEK_RESULT_SIZE 314
+static u8 readpubek[] = {
+	0, 193,			/* TPM_TAG_RQU_COMMAND */
+	0, 0, 0, 30,		/* length */
+	0, 0, 0, 124,		/* TPM_ORD_ReadPubek */
+};
+
+static ssize_t show_pubek(struct device *dev, char *buf)
+{
+	u8 data[READ_PUBEK_RESULT_SIZE];
+	ssize_t len;
+	__be32 *native_val;
+	int i;
+	char *str = buf;
+
+	struct tpm_chip *chip =
+	    pci_get_drvdata(container_of(dev, struct pci_dev, dev));
+	if (chip == NULL)
+		return -ENODEV;
+
+	memcpy(data, readpubek, sizeof(readpubek));
+	memset(data + sizeof(readpubek), 0, 20);	/* zero nonce */
+
+	if ((len = tpm_transmit(chip, data, sizeof(data))) <
+	    READ_PUBEK_RESULT_SIZE)
+		return len;
+
+	/* 
+	   ignore header 10 bytes
+	   algorithm 32 bits (1 == RSA )
+	   encscheme 16 bits
+	   sigscheme 16 bits
+	   parameters (RSA 12->bytes: keybit, #primes, expbit)  
+	   keylenbytes 32 bits
+	   256 byte modulus
+	   ignore checksum 20 bytes
+	 */
+
+	native_val = (__force __be32 *) (data + 34);
+
+	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(*native_val)
+	    );
+
+	for (i = 0; i < 256; i++) {
+		str += sprintf(str, "%02X ", data[i + 39]);
+		if ((i + 1) % 16 == 0)
+			str += sprintf(str, "\n");
+	}
+	return str - buf;
+}
+
+static DEVICE_ATTR(pubek, S_IRUGO, show_pubek, NULL);
+
+#define CAP_VER_RESULT_SIZE 18
+static u8 cap_version[] = {
+	0, 193,			/* TPM_TAG_RQU_COMMAND */
+	0, 0, 0, 18,		/* length */
+	0, 0, 0, 101,		/* TPM_ORD_GetCapability */
+	0, 0, 0, 6,
+	0, 0, 0, 0
+};
+
+#define CAP_MANUFACTURER_RESULT_SIZE 18
+static u8 cap_manufacturer[] = {
+	0, 193,			/* TPM_TAG_RQU_COMMAND */
+	0, 0, 0, 22,		/* length */
+	0, 0, 0, 101,		/* TPM_ORD_GetCapability */
+	0, 0, 0, 5,
+	0, 0, 0, 4,
+	0, 0, 1, 3
+};
+
+static ssize_t show_caps(struct device *dev, char *buf)
+{
+	u8 data[READ_PUBEK_RESULT_SIZE];
+	ssize_t len;
+	char *str = buf;
+
+	struct tpm_chip *chip =
+	    pci_get_drvdata(container_of(dev, struct pci_dev, dev));
+	if (chip == NULL)
+		return -ENODEV;
+
+	memcpy(data, cap_manufacturer, sizeof(cap_manufacturer));
+
+	if ((len = tpm_transmit(chip, data, sizeof(data))) <
+	    CAP_MANUFACTURER_RESULT_SIZE)
+		return len;
+
+	str += sprintf(str, "Manufacturer: 0x%x\n",
+		       be32_to_cpu(*(data + 14)));
+
+	memcpy(data, cap_version, sizeof(cap_version));
+
+	if ((len = tpm_transmit(chip, data, sizeof(data))) <
+	    CAP_VER_RESULT_SIZE)
+		return len;
+
+	str +=
+	    sprintf(str, "TCG version: %d.%d\nFirmware version: %d.%d\n",
+		    (int) data[14], (int) data[15], (int) data[16],
+		    (int) data[17]);
+
+	return str - buf;
+}
+
+static DEVICE_ATTR(caps, S_IRUGO, show_caps, NULL);
+
+/*
+ * Device file system interface to the TPM
+ */
+int tpm_open(struct inode *inode, struct file *file)
+{
+	int rc = 0, minor = iminor(inode);
+	struct tpm_chip *chip = NULL, *pos;
+
+	spin_lock(&driver_lock);
+
+	list_for_each_entry(pos, &tpm_chip_list, list) {
+		if (pos->vendor->miscdev.minor == minor) {
+			chip = pos;
+			break;
+		}
+	}
+
+	if (chip == NULL) {
+		rc = -ENODEV;
+		goto err_out;
+	}
+
+	if (chip->num_opens) {
+		dev_dbg(&chip->pci_dev->dev,
+			"Another process owns this TPM\n");
+		rc = -EBUSY;
+		goto err_out;
+	}
+
+	chip->num_opens++;
+	pci_dev_get(chip->pci_dev);
+
+	spin_unlock(&driver_lock);
+
+	chip->data_buffer = kmalloc(TPM_BUFSIZE * sizeof(u8), GFP_KERNEL);
+	if (chip->data_buffer == NULL) {
+		chip->num_opens--;
+		pci_dev_put(chip->pci_dev);
+		return -ENOMEM;
+	}
+
+	atomic_set(&chip->data_pending, 0);
+
+	file->private_data = chip;
+	return 0;
+
+err_out:
+	spin_unlock(&driver_lock);
+	return rc;
+}
+
+EXPORT_SYMBOL_GPL(tpm_open);
+
+int tpm_release(struct inode *inode, struct file *file)
+{
+	struct tpm_chip *chip = file->private_data;
+
+	spin_lock(&driver_lock);
+	chip->num_opens--;
+	down(&chip->timer_manipulation_mutex);
+	if (timer_pending(&chip->user_read_timer))
+		del_singleshot_timer_sync(&chip->user_read_timer);
+	else if (timer_pending(&chip->device_timer))
+		del_singleshot_timer_sync(&chip->device_timer);
+	up(&chip->timer_manipulation_mutex);
+	kfree(chip->data_buffer);
+	atomic_set(&chip->data_pending, 0);
+
+	pci_dev_put(chip->pci_dev);
+	file->private_data = NULL;
+	spin_unlock(&driver_lock);
+	return 0;
+}
+
+EXPORT_SYMBOL_GPL(tpm_release);
+
+ssize_t tpm_write(struct file * file, const char __user * buf,
+		  size_t size, loff_t * off)
+{
+	struct tpm_chip *chip = file->private_data;
+	int in_size = size, out_size;
+
+	/* cannot perform a write until the read has cleared
+	   either via tpm_read or a user_read_timer timeout */
+	while (atomic_read(&chip->data_pending) != 0) {
+		set_current_state(TASK_UNINTERRUPTIBLE);
+		schedule_timeout(TPM_TIMEOUT);
+	}
+
+	down(&chip->buffer_mutex);
+
+	if (in_size > TPM_BUFSIZE)
+		in_size = TPM_BUFSIZE;
+
+	if (copy_from_user
+	    (chip->data_buffer, (void __user *) buf, in_size)) {
+		up(&chip->buffer_mutex);
+		return -EFAULT;
+	}
+
+	/* atomic tpm command send and result receive */
+	out_size = tpm_transmit(chip, chip->data_buffer, TPM_BUFSIZE);
+
+	atomic_set(&chip->data_pending, out_size);
+	up(&chip->buffer_mutex);
+
+	/* Set a timeout by which the reader must come claim the result */
+	down(&chip->timer_manipulation_mutex);
+	init_timer(&chip->user_read_timer);
+	chip->user_read_timer.function = user_reader_timeout;
+	chip->user_read_timer.data = (unsigned long) chip;
+	chip->user_read_timer.expires = jiffies + (60 * HZ);
+	add_timer(&chip->user_read_timer);
+	up(&chip->timer_manipulation_mutex);
+
+	return in_size;
+}
+
+EXPORT_SYMBOL_GPL(tpm_write);
+
+ssize_t tpm_read(struct file * file, char __user * buf,
+		 size_t size, loff_t * off)
+{
+	struct tpm_chip *chip = file->private_data;
+	int ret_size = -ENODATA;
+
+	if (atomic_read(&chip->data_pending) != 0) {	/* Result available */
+		down(&chip->timer_manipulation_mutex);
+		del_singleshot_timer_sync(&chip->user_read_timer);
+		up(&chip->timer_manipulation_mutex);
+
+		down(&chip->buffer_mutex);
+
+		ret_size = atomic_read(&chip->data_pending);
+		atomic_set(&chip->data_pending, 0);
+
+		if (ret_size == 0)	/* timeout just occurred */
+			ret_size = -ETIME;
+		else if (ret_size > 0) {	/* relay data */
+			if (size < ret_size)
+				ret_size = size;
+
+			if (copy_to_user((void __user *) buf,
+					 chip->data_buffer, ret_size)) {
+				ret_size = -EFAULT;
+			}
+		}
+		up(&chip->buffer_mutex);
+	}
+
+	return ret_size;
+}
+
+EXPORT_SYMBOL_GPL(tpm_read);
+
+void __devexit tpm_remove(struct pci_dev *pci_dev)
+{
+	struct tpm_chip *chip = pci_get_drvdata(pci_dev);
+
+	if (chip == NULL) {
+		dev_err(&pci_dev->dev, "No device data found\n");
+		return;
+	}
+
+	spin_lock(&driver_lock);
+
+	list_del(&chip->list);
+
+	pci_set_drvdata(pci_dev, NULL);
+	misc_deregister(&chip->vendor->miscdev);
+
+	device_remove_file(&pci_dev->dev, &dev_attr_pubek);
+	device_remove_file(&pci_dev->dev, &dev_attr_pcrs);
+	device_remove_file(&pci_dev->dev, &dev_attr_caps);
+
+	spin_unlock(&driver_lock);
+
+	pci_disable_device(pci_dev);
+
+	dev_mask[chip->dev_num / 32] &= !(1 << (chip->dev_num % 32));
+
+	kfree(chip);
+
+	pci_dev_put(pci_dev);
+}
+
+EXPORT_SYMBOL_GPL(tpm_remove);
+
+static u8 savestate[] = {
+	0, 193,			/* TPM_TAG_RQU_COMMAND */
+	0, 0, 0, 10,		/* blob length (in bytes) */
+	0, 0, 0, 152		/* TPM_ORD_SaveState */
+};
+
+/*
+ * We are about to suspend. Save the TPM state
+ * so that it can be restored.
+ */
+int tpm_pm_suspend(struct pci_dev *pci_dev, u32 pm_state)
+{
+	struct tpm_chip *chip = pci_get_drvdata(pci_dev);
+	if (chip == NULL)
+		return -ENODEV;
+
+	tpm_transmit(chip, savestate, sizeof(savestate));
+	return 0;
+}
+
+EXPORT_SYMBOL_GPL(tpm_pm_suspend);
+
+/*
+ * Resume from a power safe. The BIOS already restored
+ * the TPM state.
+ */
+int tpm_pm_resume(struct pci_dev *pci_dev)
+{
+	struct tpm_chip *chip = pci_get_drvdata(pci_dev);
+	if (chip == NULL)
+		return -ENODEV;
+
+	spin_lock(&driver_lock);
+	tpm_lpc_bus_init(pci_dev, chip->vendor->base);
+	spin_unlock(&driver_lock);
+
+	return 0;
+}
+
+EXPORT_SYMBOL_GPL(tpm_pm_resume);
+
+/*
+ * Called from tpm_<specific>.c probe function only for devices 
+ * the driver has determined it should claim.  Prior to calling
+ * this function the specific probe function has called pci_enable_device
+ * upon errant exit from this function specific probe function should call
+ * pci_disable_device
+ */
+int tpm_register_hardware(struct pci_dev *pci_dev,
+			  struct tpm_vendor_specific *entry)
+{
+	char devname[7];
+	struct tpm_chip *chip;
+	int i, j;
+
+	/* Driver specific per-device data */
+	chip = kmalloc(sizeof(*chip), GFP_KERNEL);
+	if (chip == NULL)
+		return -ENOMEM;
+
+	memset(chip, 0, sizeof(struct tpm_chip));
+
+	init_MUTEX(&chip->buffer_mutex);
+	init_MUTEX(&chip->tpm_mutex);
+	init_MUTEX(&chip->timer_manipulation_mutex);
+	INIT_LIST_HEAD(&chip->list);
+
+	chip->vendor = entry;
+
+	chip->dev_num = -1;
+
+	for (i = 0; i < 32; i++)
+		for (j = 0; j < 8; j++)
+			if ((dev_mask[i] & (1 << j)) == 0) {
+				chip->dev_num = i * 32 + j;
+				dev_mask[i] |= 1 << j;
+				goto dev_num_search_complete;
+			}
+
+dev_num_search_complete:
+	if (chip->dev_num < 0) {
+		dev_err(&pci_dev->dev,
+			"No available tpm device numbers\n");
+		kfree(chip);
+		return -ENODEV;
+	} else if (chip->dev_num == 0)
+		chip->vendor->miscdev.minor = TPM_MINOR;
+	else
+		chip->vendor->miscdev.minor = MISC_DYNAMIC_MINOR;
+
+	snprintf(devname, sizeof(devname), "%s%d", "tpm", chip->dev_num);
+	chip->vendor->miscdev.name = devname;
+
+	chip->vendor->miscdev.dev = &(pci_dev->dev);
+	chip->pci_dev = pci_dev_get(pci_dev);
+
+	spin_lock(&driver_lock);
+
+	if (misc_register(&chip->vendor->miscdev)) {
+		dev_err(&chip->pci_dev->dev,
+			"unable to misc_register %s, minor %d\n",
+			chip->vendor->miscdev.name,
+			chip->vendor->miscdev.minor);
+		pci_dev_put(pci_dev);
+		spin_unlock(&driver_lock);
+		kfree(chip);
+		dev_mask[i] &= !(1 << j);
+		return -ENODEV;
+	}
+
+	pci_set_drvdata(pci_dev, chip);
+
+	list_add(&chip->list, &tpm_chip_list);
+
+	device_create_file(&pci_dev->dev, &dev_attr_pubek);
+	device_create_file(&pci_dev->dev, &dev_attr_pcrs);
+	device_create_file(&pci_dev->dev, &dev_attr_caps);
+
+	spin_unlock(&driver_lock);
+	return 0;
+}
+
+EXPORT_SYMBOL_GPL(tpm_register_hardware);
+
+static int __init init_tpm(void)
+{
+	return 0;
+}
+
+static void __exit cleanup_tpm(void)
+{
+
+}
+
+module_init(init_tpm);
+module_exit(cleanup_tpm);
+
+MODULE_AUTHOR("Leendert van Doorn (leendert@watson.ibm.com)");
+MODULE_DESCRIPTION("TPM Driver");
+MODULE_VERSION("2.0");
+MODULE_LICENSE("GPL");
diff -Nru a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
--- /dev/null	Wed Dec 31 16:00:00 196900
+++ b/drivers/char/tpm/tpm.h	2005-03-09 16:40:26 -08:00
@@ -0,0 +1,92 @@
+/*
+ * Copyright (C) 2004 IBM Corporation
+ *
+ * Authors:
+ * Leendert van Doorn <leendert@watson.ibm.com>
+ * Dave Safford <safford@watson.ibm.com>
+ * Reiner Sailer <sailer@watson.ibm.com>
+ * Kylene Hall <kjhall@us.ibm.com>
+ *
+ * Maintained by: <tpmdd_devel@lists.sourceforge.net>
+ *
+ * Device driver for TCG/TCPA TPM (trusted platform module).
+ * Specifications at www.trustedcomputinggroup.org	 
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation, version 2 of the
+ * License.
+ * 
+ */
+#include <linux/module.h>
+#include <linux/version.h>
+#include <linux/pci.h>
+#include <linux/delay.h>
+#include <linux/miscdevice.h>
+
+#define TPM_TIMEOUT msecs_to_jiffies(5)
+
+/* TPM addresses */
+#define	TPM_ADDR			0x4E
+#define	TPM_DATA			0x4F
+
+struct tpm_chip;
+
+struct tpm_vendor_specific {
+	u8 req_complete_mask;
+	u8 req_complete_val;
+	u16 base;		/* TPM base address */
+
+	int (*recv) (struct tpm_chip *, u8 *, size_t);
+	int (*send) (struct tpm_chip *, u8 *, size_t);
+	void (*cancel) (struct tpm_chip *);
+	struct miscdevice miscdev;
+};
+
+struct tpm_chip {
+	struct pci_dev *pci_dev;	/* PCI device stuff */
+
+	int dev_num;		/* /dev/tpm# */
+	int num_opens;		/* only one allowed */
+	int time_expired;
+
+	/* Data passed to and from the tpm via the read/write calls */
+	u8 *data_buffer;
+	atomic_t data_pending;
+	struct semaphore buffer_mutex;
+
+	struct timer_list user_read_timer;	/* user needs to claim result */
+	struct semaphore tpm_mutex;	/* tpm is processing */
+	struct timer_list device_timer;	/* tpm is processing */
+	struct semaphore timer_manipulation_mutex;
+
+	struct tpm_vendor_specific *vendor;
+
+	struct list_head list;
+};
+
+static inline int tpm_read_index(int index)
+{
+	outb(index, TPM_ADDR);
+	return inb(TPM_DATA) & 0xFF;
+}
+
+static inline void tpm_write_index(int index, int value)
+{
+	outb(index, TPM_ADDR);
+	outb(value & 0xFF, TPM_DATA);
+}
+
+extern void tpm_time_expired(unsigned long);
+extern int tpm_lpc_bus_init(struct pci_dev *, u16);
+
+extern int tpm_register_hardware(struct pci_dev *,
+				 struct tpm_vendor_specific *);
+extern int tpm_open(struct inode *, struct file *);
+extern int tpm_release(struct inode *, struct file *);
+extern ssize_t tpm_write(struct file *, const char __user *, size_t,
+			 loff_t *);
+extern ssize_t tpm_read(struct file *, char __user *, size_t, loff_t *);
+extern void __devexit tpm_remove(struct pci_dev *);
+extern int tpm_pm_suspend(struct pci_dev *, u32);
+extern int tpm_pm_resume(struct pci_dev *);
diff -Nru a/drivers/char/tpm/tpm_atmel.c b/drivers/char/tpm/tpm_atmel.c
--- /dev/null	Wed Dec 31 16:00:00 196900
+++ b/drivers/char/tpm/tpm_atmel.c	2005-03-09 16:40:26 -08:00
@@ -0,0 +1,216 @@
+/*
+ * Copyright (C) 2004 IBM Corporation
+ *
+ * Authors:
+ * Leendert van Doorn <leendert@watson.ibm.com>
+ * Dave Safford <safford@watson.ibm.com>
+ * Reiner Sailer <sailer@watson.ibm.com>
+ * Kylene Hall <kjhall@us.ibm.com>
+ *
+ * Maintained by: <tpmdd_devel@lists.sourceforge.net>
+ *
+ * Device driver for TCG/TCPA TPM (trusted platform module).
+ * Specifications at www.trustedcomputinggroup.org	 
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation, version 2 of the
+ * License.
+ * 
+ */
+
+#include "tpm.h"
+
+/* Atmel definitions */
+#define	TPM_ATML_BASE			0x400
+
+/* write status bits */
+#define	ATML_STATUS_ABORT		0x01
+#define	ATML_STATUS_LASTBYTE		0x04
+
+/* read status bits */
+#define	ATML_STATUS_BUSY		0x01
+#define	ATML_STATUS_DATA_AVAIL		0x02
+#define	ATML_STATUS_REWRITE		0x04
+
+
+static int tpm_atml_recv(struct tpm_chip *chip, u8 * buf, size_t count)
+{
+	u8 status, *hdr = buf;
+	u32 size;
+	int i;
+	__be32 *native_size;
+
+	/* start reading header */
+	if (count < 6)
+		return -EIO;
+
+	for (i = 0; i < 6; i++) {
+		status = inb(chip->vendor->base + 1);
+		if ((status & ATML_STATUS_DATA_AVAIL) == 0) {
+			dev_err(&chip->pci_dev->dev,
+				"error reading header\n");
+			return -EIO;
+		}
+		*buf++ = inb(chip->vendor->base);
+	}
+
+	/* size of the data received */
+	native_size = (__force __be32 *) (hdr + 2);
+	size = be32_to_cpu(*native_size);
+
+	if (count < size) {
+		dev_err(&chip->pci_dev->dev,
+			"Recv size(%d) less than available space\n", size);
+		for (; i < size; i++) {	/* clear the waiting data anyway */
+			status = inb(chip->vendor->base + 1);
+			if ((status & ATML_STATUS_DATA_AVAIL) == 0) {
+				dev_err(&chip->pci_dev->dev,
+					"error reading data\n");
+				return -EIO;
+			}
+		}
+		return -EIO;
+	}
+
+	/* read all the data available */
+	for (; i < size; i++) {
+		status = inb(chip->vendor->base + 1);
+		if ((status & ATML_STATUS_DATA_AVAIL) == 0) {
+			dev_err(&chip->pci_dev->dev,
+				"error reading data\n");
+			return -EIO;
+		}
+		*buf++ = inb(chip->vendor->base);
+	}
+
+	/* make sure data available is gone */
+	status = inb(chip->vendor->base + 1);
+	if (status & ATML_STATUS_DATA_AVAIL) {
+		dev_err(&chip->pci_dev->dev, "data available is stuck\n");
+		return -EIO;
+	}
+
+	return size;
+}
+
+static int tpm_atml_send(struct tpm_chip *chip, u8 * buf, size_t count)
+{
+	int i;
+
+	dev_dbg(&chip->pci_dev->dev, "tpm_atml_send: ");
+	for (i = 0; i < count; i++) {
+		dev_dbg(&chip->pci_dev->dev, "0x%x(%d) ", buf[i], buf[i]);
+		outb(buf[i], chip->vendor->base);
+	}
+
+	return count;
+}
+
+static void tpm_atml_cancel(struct tpm_chip *chip)
+{
+	outb(ATML_STATUS_ABORT, chip->vendor->base + 1);
+}
+
+static struct file_operations atmel_ops = {
+	.owner = THIS_MODULE,
+	.llseek = no_llseek,
+	.open = tpm_open,
+	.read = tpm_read,
+	.write = tpm_write,
+	.release = tpm_release,
+};
+
+static struct tpm_vendor_specific tpm_atmel = {
+	.recv = tpm_atml_recv,
+	.send = tpm_atml_send,
+	.cancel = tpm_atml_cancel,
+	.req_complete_mask = ATML_STATUS_BUSY | ATML_STATUS_DATA_AVAIL,
+	.req_complete_val = ATML_STATUS_DATA_AVAIL,
+	.base = TPM_ATML_BASE,
+	.miscdev.fops = &atmel_ops,
+};
+
+static int __devinit tpm_atml_init(struct pci_dev *pci_dev,
+				   const struct pci_device_id *pci_id)
+{
+	u8 version[4];
+	int rc = 0;
+
+	if (pci_enable_device(pci_dev))
+		return -EIO;
+
+	if (tpm_lpc_bus_init(pci_dev, TPM_ATML_BASE)) {
+		rc = -ENODEV;
+		goto out_err;
+	}
+
+	/* verify that it is an Atmel part */
+	if (tpm_read_index(4) != 'A' || tpm_read_index(5) != 'T'
+	    || tpm_read_index(6) != 'M' || tpm_read_index(7) != 'L') {
+		rc = -ENODEV;
+		goto out_err;
+	}
+
+	/* query chip for its version number */
+	if ((version[0] = tpm_read_index(0x00)) != 0xFF) {
+		version[1] = tpm_read_index(0x01);
+		version[2] = tpm_read_index(0x02);
+		version[3] = tpm_read_index(0x03);
+	} else {
+		dev_info(&pci_dev->dev, "version query failed\n");
+		rc = -ENODEV;
+		goto out_err;
+	}
+
+	if ((rc = tpm_register_hardware(pci_dev, &tpm_atmel)) < 0)
+		goto out_err;
+
+	dev_info(&pci_dev->dev,
+		 "Atmel TPM version %d.%d.%d.%d\n", version[0], version[1],
+		 version[2], version[3]);
+
+	return 0;
+out_err:
+	pci_disable_device(pci_dev);
+	return rc;
+}
+
+static struct pci_device_id tpm_pci_tbl[] __devinitdata = {
+	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82801BA_0)},
+	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82801CA_12)},
+	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82801DB_0)},
+	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82801DB_12)},
+	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82801EB_0)},
+	{PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_8111_LPC)},
+	{0,}
+};
+
+MODULE_DEVICE_TABLE(pci, tpm_pci_tbl);
+
+static struct pci_driver atmel_pci_driver = {
+	.name = "tpm_atmel",
+	.id_table = tpm_pci_tbl,
+	.probe = tpm_atml_init,
+	.remove = __devexit_p(tpm_remove),
+	.suspend = tpm_pm_suspend,
+	.resume = tpm_pm_resume,
+};
+
+static int __init init_atmel(void)
+{
+	return pci_register_driver(&atmel_pci_driver);
+}
+
+static void __exit cleanup_atmel(void)
+{
+	pci_unregister_driver(&atmel_pci_driver);
+}
+
+module_init(init_atmel);
+module_exit(cleanup_atmel);
+
+MODULE_AUTHOR("Leendert van Doorn (leendert@watson.ibm.com)");
+MODULE_DESCRIPTION("TPM Driver");
+MODULE_VERSION("2.0");
+MODULE_LICENSE("GPL");
diff -Nru a/drivers/char/tpm/tpm_nsc.c b/drivers/char/tpm/tpm_nsc.c
--- /dev/null	Wed Dec 31 16:00:00 196900
+++ b/drivers/char/tpm/tpm_nsc.c	2005-03-09 16:40:26 -08:00
@@ -0,0 +1,372 @@
+/*
+ * Copyright (C) 2004 IBM Corporation
+ *
+ * Authors:
+ * Leendert van Doorn <leendert@watson.ibm.com>
+ * Dave Safford <safford@watson.ibm.com>
+ * Reiner Sailer <sailer@watson.ibm.com>
+ * Kylene Hall <kjhall@us.ibm.com>
+ *
+ * Maintained by: <tpmdd_devel@lists.sourceforge.net>
+ *
+ * Device driver for TCG/TCPA TPM (trusted platform module).
+ * Specifications at www.trustedcomputinggroup.org	 
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation, version 2 of the
+ * License.
+ * 
+ */
+
+#include "tpm.h"
+
+/* National definitions */
+#define	TPM_NSC_BASE			0x360
+#define	TPM_NSC_IRQ			0x07
+
+#define	NSC_LDN_INDEX			0x07
+#define	NSC_SID_INDEX			0x20
+#define	NSC_LDC_INDEX			0x30
+#define	NSC_DIO_INDEX			0x60
+#define	NSC_CIO_INDEX			0x62
+#define	NSC_IRQ_INDEX			0x70
+#define	NSC_ITS_INDEX			0x71
+
+#define	NSC_STATUS			0x01
+#define	NSC_COMMAND			0x01
+#define	NSC_DATA			0x00
+
+/* status bits */
+#define	NSC_STATUS_OBF			0x01	/* output buffer full */
+#define	NSC_STATUS_IBF			0x02	/* input buffer full */
+#define	NSC_STATUS_F0			0x04	/* F0 */
+#define	NSC_STATUS_A2			0x08	/* A2 */
+#define	NSC_STATUS_RDY			0x10	/* ready to receive command */
+#define	NSC_STATUS_IBR			0x20	/* ready to receive data */
+
+/* command bits */
+#define	NSC_COMMAND_NORMAL		0x01	/* normal mode */
+#define	NSC_COMMAND_EOC			0x03
+#define	NSC_COMMAND_CANCEL		0x22
+
+/*
+ * Wait for a certain status to appear
+ */
+static int wait_for_stat(struct tpm_chip *chip, u8 mask, u8 val, u8 * data)
+{
+	int expired = 0;
+	struct timer_list status_timer =
+	    TIMER_INITIALIZER(tpm_time_expired, jiffies + 10 * HZ,
+			      (unsigned long) &expired);
+
+	/* status immediately available check */
+	*data = inb(chip->vendor->base + NSC_STATUS);
+	if ((*data & mask) == val)
+		return 0;
+
+	/* wait for status */
+	add_timer(&status_timer);
+	do {
+		set_current_state(TASK_UNINTERRUPTIBLE);
+		schedule_timeout(TPM_TIMEOUT);
+		*data = inb(chip->vendor->base + 1);
+		if ((*data & mask) == val) {
+			del_singleshot_timer_sync(&status_timer);
+			return 0;
+		}
+	}
+	while (!expired);
+
+	return -EBUSY;
+}
+
+static int nsc_wait_for_ready(struct tpm_chip *chip)
+{
+	int status;
+	int expired = 0;
+	struct timer_list status_timer =
+	    TIMER_INITIALIZER(tpm_time_expired, jiffies + 100,
+			      (unsigned long) &expired);
+
+	/* status immediately available check */
+	status = inb(chip->vendor->base + NSC_STATUS);
+	if (status & NSC_STATUS_OBF)
+		status = inb(chip->vendor->base + NSC_DATA);
+	if (status & NSC_STATUS_RDY)
+		return 0;
+
+	/* wait for status */
+	add_timer(&status_timer);
+	do {
+		set_current_state(TASK_UNINTERRUPTIBLE);
+		schedule_timeout(TPM_TIMEOUT);
+		status = inb(chip->vendor->base + NSC_STATUS);
+		if (status & NSC_STATUS_OBF)
+			status = inb(chip->vendor->base + NSC_DATA);
+		if (status & NSC_STATUS_RDY) {
+			del_singleshot_timer_sync(&status_timer);
+			return 0;
+		}
+	}
+	while (!expired);
+
+	dev_info(&chip->pci_dev->dev, "wait for ready failed\n");
+	return -EBUSY;
+}
+
+
+static int tpm_nsc_recv(struct tpm_chip *chip, u8 * buf, size_t count)
+{
+	u8 *buffer = buf;
+	u8 data, *p;
+	u32 size;
+	__be32 *native_size;
+
+	if (count < 6)
+		return -EIO;
+
+	if (wait_for_stat(chip, NSC_STATUS_F0, NSC_STATUS_F0, &data) < 0) {
+		dev_err(&chip->pci_dev->dev, "F0 timeout\n");
+		return -EIO;
+	}
+	if ((data =
+	     inb(chip->vendor->base + NSC_DATA)) != NSC_COMMAND_NORMAL) {
+		dev_err(&chip->pci_dev->dev, "not in normal mode (0x%x)\n",
+			data);
+		return -EIO;
+	}
+
+	/* read the whole packet */
+	for (p = buffer; p < &buffer[count]; p++) {
+		if (wait_for_stat
+		    (chip, NSC_STATUS_OBF, NSC_STATUS_OBF, &data) < 0) {
+			dev_err(&chip->pci_dev->dev,
+				"OBF timeout (while reading data)\n");
+			return -EIO;
+		}
+		if (data & NSC_STATUS_F0)
+			break;
+		*p = inb(chip->vendor->base + NSC_DATA);
+	}
+
+	if ((data & NSC_STATUS_F0) == 0) {
+		dev_err(&chip->pci_dev->dev, "F0 not set\n");
+		return -EIO;
+	}
+	if ((data = inb(chip->vendor->base + NSC_DATA)) != NSC_COMMAND_EOC) {
+		dev_err(&chip->pci_dev->dev,
+			"expected end of command(0x%x)\n", data);
+		return -EIO;
+	}
+
+	native_size = (__force __be32 *) (buf + 2);
+	size = be32_to_cpu(*native_size);
+
+	if (count < size)
+		return -EIO;
+
+	return size;
+}
+
+static int tpm_nsc_send(struct tpm_chip *chip, u8 * buf, size_t count)
+{
+	u8 data;
+	int i;
+
+	/*
+	 * If we hit the chip with back to back commands it locks up
+	 * and never set IBF. Hitting it with this "hammer" seems to
+	 * fix it. Not sure why this is needed, we followed the flow
+	 * chart in the manual to the letter.
+	 */
+	outb(NSC_COMMAND_CANCEL, chip->vendor->base + NSC_COMMAND);
+
+	if (nsc_wait_for_ready(chip) != 0)
+		return -EIO;
+
+	if (wait_for_stat(chip, NSC_STATUS_IBF, 0, &data) < 0) {
+		dev_err(&chip->pci_dev->dev, "IBF timeout\n");
+		return -EIO;
+	}
+
+	outb(NSC_COMMAND_NORMAL, chip->vendor->base + NSC_COMMAND);
+	if (wait_for_stat(chip, NSC_STATUS_IBR, NSC_STATUS_IBR, &data) < 0) {
+		dev_err(&chip->pci_dev->dev, "IBR timeout\n");
+		return -EIO;
+	}
+
+	for (i = 0; i < count; i++) {
+		if (wait_for_stat(chip, NSC_STATUS_IBF, 0, &data) < 0) {
+			dev_err(&chip->pci_dev->dev,
+				"IBF timeout (while writing data)\n");
+			return -EIO;
+		}
+		outb(buf[i], chip->vendor->base + NSC_DATA);
+	}
+
+	if (wait_for_stat(chip, NSC_STATUS_IBF, 0, &data) < 0) {
+		dev_err(&chip->pci_dev->dev, "IBF timeout\n");
+		return -EIO;
+	}
+	outb(NSC_COMMAND_EOC, chip->vendor->base + NSC_COMMAND);
+
+	return count;
+}
+
+static void tpm_nsc_cancel(struct tpm_chip *chip)
+{
+	outb(NSC_COMMAND_CANCEL, chip->vendor->base + NSC_COMMAND);
+}
+
+static struct file_operations nsc_ops = {
+	.owner = THIS_MODULE,
+	.llseek = no_llseek,
+	.open = tpm_open,
+	.read = tpm_read,
+	.write = tpm_write,
+	.release = tpm_release,
+};
+
+static struct tpm_vendor_specific tpm_nsc = {
+	.recv = tpm_nsc_recv,
+	.send = tpm_nsc_send,
+	.cancel = tpm_nsc_cancel,
+	.req_complete_mask = NSC_STATUS_OBF,
+	.req_complete_val = NSC_STATUS_OBF,
+	.base = TPM_NSC_BASE,
+	.miscdev.fops = &nsc_ops,
+};
+
+static int __devinit tpm_nsc_init(struct pci_dev *pci_dev,
+				  const struct pci_device_id *pci_id)
+{
+	int rc = 0;
+
+	if (pci_enable_device(pci_dev))
+		return -EIO;
+
+	if (tpm_lpc_bus_init(pci_dev, TPM_NSC_BASE)) {
+		rc = -ENODEV;
+		goto out_err;
+	}
+
+	/* verify that it is a National part (SID) */
+	if (tpm_read_index(NSC_SID_INDEX) != 0xEF) {
+		rc = -ENODEV;
+		goto out_err;
+	}
+
+	dev_dbg(&pci_dev->dev, "NSC TPM detected\n");
+	dev_dbg(&pci_dev->dev,
+		"NSC LDN 0x%x, SID 0x%x, SRID 0x%x\n",
+		tpm_read_index(0x07), tpm_read_index(0x20),
+		tpm_read_index(0x27));
+	dev_dbg(&pci_dev->dev,
+		"NSC SIOCF1 0x%x SIOCF5 0x%x SIOCF6 0x%x SIOCF8 0x%x\n",
+		tpm_read_index(0x21), tpm_read_index(0x25),
+		tpm_read_index(0x26), tpm_read_index(0x28));
+	dev_dbg(&pci_dev->dev, "NSC IO Base0 0x%x\n",
+		(tpm_read_index(0x60) << 8) | tpm_read_index(0x61));
+	dev_dbg(&pci_dev->dev, "NSC IO Base1 0x%x\n",
+		(tpm_read_index(0x62) << 8) | tpm_read_index(0x63));
+	dev_dbg(&pci_dev->dev, "NSC Interrupt number and wakeup 0x%x\n",
+		tpm_read_index(0x70));
+	dev_dbg(&pci_dev->dev, "NSC IRQ type select 0x%x\n",
+		tpm_read_index(0x71));
+	dev_dbg(&pci_dev->dev,
+		"NSC DMA channel select0 0x%x, select1 0x%x\n",
+		tpm_read_index(0x74), tpm_read_index(0x75));
+	dev_dbg(&pci_dev->dev,
+		"NSC Config "
+		"0x%x 0x%x 0x%x 0x%x 0x%x 0x%x 0x%x 0x%x 0x%x 0x%x\n",
+		tpm_read_index(0xF0), tpm_read_index(0xF1),
+		tpm_read_index(0xF2), tpm_read_index(0xF3),
+		tpm_read_index(0xF4), tpm_read_index(0xF5),
+		tpm_read_index(0xF6), tpm_read_index(0xF7),
+		tpm_read_index(0xF8), tpm_read_index(0xF9));
+
+	dev_info(&pci_dev->dev,
+		 "NSC PC21100 TPM revision %d\n",
+		 tpm_read_index(0x27) & 0x1F);
+
+	if (tpm_read_index(NSC_LDC_INDEX) == 0)
+		dev_info(&pci_dev->dev, ": NSC TPM not active\n");
+
+	/* select PM channel 1 */
+	tpm_write_index(NSC_LDN_INDEX, 0x12);
+	tpm_read_index(NSC_LDN_INDEX);
+
+	/* disable the DPM module */
+	tpm_write_index(NSC_LDC_INDEX, 0);
+	tpm_read_index(NSC_LDC_INDEX);
+
+	/* set the data register base addresses */
+	tpm_write_index(NSC_DIO_INDEX, TPM_NSC_BASE >> 8);
+	tpm_write_index(NSC_DIO_INDEX + 1, TPM_NSC_BASE);
+	tpm_read_index(NSC_DIO_INDEX);
+	tpm_read_index(NSC_DIO_INDEX + 1);
+
+	/* set the command register base addresses */
+	tpm_write_index(NSC_CIO_INDEX, (TPM_NSC_BASE + 1) >> 8);
+	tpm_write_index(NSC_CIO_INDEX + 1, (TPM_NSC_BASE + 1));
+	tpm_read_index(NSC_DIO_INDEX);
+	tpm_read_index(NSC_DIO_INDEX + 1);
+
+	/* set the interrupt number to be used for the host interface */
+	tpm_write_index(NSC_IRQ_INDEX, TPM_NSC_IRQ);
+	tpm_write_index(NSC_ITS_INDEX, 0x00);
+	tpm_read_index(NSC_IRQ_INDEX);
+
+	/* enable the DPM module */
+	tpm_write_index(NSC_LDC_INDEX, 0x01);
+	tpm_read_index(NSC_LDC_INDEX);
+
+	if ((rc = tpm_register_hardware(pci_dev, &tpm_nsc)) < 0)
+		goto out_err;
+
+	return 0;
+
+out_err:
+	pci_disable_device(pci_dev);
+	return rc;
+}
+
+static struct pci_device_id tpm_pci_tbl[] __devinitdata = {
+	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82801BA_0)},
+	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82801CA_12)},
+	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82801DB_0)},
+	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82801DB_12)},
+	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82801EB_0)},
+	{PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_8111_LPC)},
+	{0,}
+};
+
+MODULE_DEVICE_TABLE(pci, tpm_pci_tbl);
+
+static struct pci_driver nsc_pci_driver = {
+	.name = "tpm_nsc",
+	.id_table = tpm_pci_tbl,
+	.probe = tpm_nsc_init,
+	.remove = __devexit_p(tpm_remove),
+	.suspend = tpm_pm_suspend,
+	.resume = tpm_pm_resume,
+};
+
+static int __init init_nsc(void)
+{
+	return pci_register_driver(&nsc_pci_driver);
+}
+
+static void __exit cleanup_nsc(void)
+{
+	pci_unregister_driver(&nsc_pci_driver);
+}
+
+module_init(init_nsc);
+module_exit(cleanup_nsc);
+
+MODULE_AUTHOR("Leendert van Doorn (leendert@watson.ibm.com)");
+MODULE_DESCRIPTION("TPM Driver");
+MODULE_VERSION("2.0");
+MODULE_LICENSE("GPL");
diff -Nru a/include/linux/pci_ids.h b/include/linux/pci_ids.h
--- a/include/linux/pci_ids.h	2005-03-09 16:40:26 -08:00
+++ b/include/linux/pci_ids.h	2005-03-09 16:40:26 -08:00
@@ -519,6 +519,7 @@
 #define PCI_DEVICE_ID_AMD_OPUS_7449	0x7449
 #	define PCI_DEVICE_ID_AMD_VIPER_7449	PCI_DEVICE_ID_AMD_OPUS_7449
 #define PCI_DEVICE_ID_AMD_8111_LAN	0x7462
+#define PCI_DEVICE_ID_AMD_8111_LPC	0x7468
 #define PCI_DEVICE_ID_AMD_8111_IDE	0x7469
 #define PCI_DEVICE_ID_AMD_8111_SMBUS2	0x746a
 #define PCI_DEVICE_ID_AMD_8111_SMBUS	0x746b

[PATCH] tpm: fix cause of SMP stack traces

[Greg KH]

ChangeSet 1.2036, 2005/03/09 10:12:38-08:00, kjhall@us.ibm.com

[PATCH] tpm: fix cause of SMP stack traces

There were misplaced spinlock acquires and releases in the probe, close and release
paths which were causing might_sleep and schedule while atomic error messages accompanied
by stack traces when the kernel was compiled with SMP support. Bug reported by Reben Jenster
<ruben@hotheads.de>

Signed-off-by: Kylene Hall <kjhall@us.ibm.com>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>


 drivers/char/tpm/tpm.c |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)


diff -Nru a/drivers/char/tpm/tpm.c b/drivers/char/tpm/tpm.c
--- a/drivers/char/tpm/tpm.c	2005-03-09 16:40:19 -08:00
+++ b/drivers/char/tpm/tpm.c	2005-03-09 16:40:19 -08:00
@@ -422,21 +422,24 @@
 int tpm_release(struct inode *inode, struct file *file)
 {
 	struct tpm_chip *chip = file->private_data;
+	
+	file->private_data = NULL;
 
 	spin_lock(&driver_lock);
 	chip->num_opens--;
+	spin_unlock(&driver_lock);
+
 	down(&chip->timer_manipulation_mutex);
 	if (timer_pending(&chip->user_read_timer))
 		del_singleshot_timer_sync(&chip->user_read_timer);
 	else if (timer_pending(&chip->device_timer))
 		del_singleshot_timer_sync(&chip->device_timer);
 	up(&chip->timer_manipulation_mutex);
+
 	kfree(chip->data_buffer);
 	atomic_set(&chip->data_pending, 0);
 
 	pci_dev_put(chip->pci_dev);
-	file->private_data = NULL;
-	spin_unlock(&driver_lock);
 	return 0;
 }
 
@@ -534,6 +537,8 @@
 
 	list_del(&chip->list);
 
+	spin_unlock(&driver_lock);
+
 	pci_set_drvdata(pci_dev, NULL);
 	misc_deregister(&chip->vendor->miscdev);
 
@@ -541,8 +546,6 @@
 	device_remove_file(&pci_dev->dev, &dev_attr_pcrs);
 	device_remove_file(&pci_dev->dev, &dev_attr_caps);
 
-	spin_unlock(&driver_lock);
-
 	pci_disable_device(pci_dev);
 
 	dev_mask[chip->dev_num / 32] &= !(1 << (chip->dev_num % 32));
@@ -583,6 +586,7 @@
 int tpm_pm_resume(struct pci_dev *pci_dev)
 {
 	struct tpm_chip *chip = pci_get_drvdata(pci_dev);
+
 	if (chip == NULL)
 		return -ENODEV;
 
@@ -650,15 +654,12 @@
 	chip->vendor->miscdev.dev = &(pci_dev->dev);
 	chip->pci_dev = pci_dev_get(pci_dev);
 
-	spin_lock(&driver_lock);
-
 	if (misc_register(&chip->vendor->miscdev)) {
 		dev_err(&chip->pci_dev->dev,
 			"unable to misc_register %s, minor %d\n",
 			chip->vendor->miscdev.name,
 			chip->vendor->miscdev.minor);
 		pci_dev_put(pci_dev);
-		spin_unlock(&driver_lock);
 		kfree(chip);
 		dev_mask[i] &= !(1 << j);
 		return -ENODEV;
@@ -672,7 +673,6 @@
 	device_create_file(&pci_dev->dev, &dev_attr_pcrs);
 	device_create_file(&pci_dev->dev, &dev_attr_caps);
 
-	spin_unlock(&driver_lock);
 	return 0;
 }
 

[PATCH] tpm_msc-build-fix

[Greg KH]

ChangeSet 1.2037, 2005/03/09 10:12:56-08:00, akpm@osdl.org

[PATCH] tpm_msc-build-fix

With older gcc's:

drivers/char/tpm/tpm_nsc.c:238: unknown field `fops' specified in initializer
drivers/char/tpm/tpm_nsc.c:238: warning: missing braces around initializer


Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>


 drivers/char/tpm/tpm_nsc.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletion(-)


diff -Nru a/drivers/char/tpm/tpm_nsc.c b/drivers/char/tpm/tpm_nsc.c
--- a/drivers/char/tpm/tpm_nsc.c	2005-03-09 16:40:12 -08:00
+++ b/drivers/char/tpm/tpm_nsc.c	2005-03-09 16:40:12 -08:00
@@ -235,7 +235,8 @@
 	.req_complete_mask = NSC_STATUS_OBF,
 	.req_complete_val = NSC_STATUS_OBF,
 	.base = TPM_NSC_BASE,
-	.miscdev.fops = &nsc_ops,
+	.miscdev = { .fops = &nsc_ops, },
+	
 };
 
 static int __devinit tpm_nsc_init(struct pci_dev *pci_dev,

[PATCH] tpm_atmel build fix

[Greg KH]

ChangeSet 1.2038, 2005/03/09 10:13:15-08:00, akpm@osdl.org

[PATCH] tpm_atmel build fix

drivers/char/tpm/tpm_atmel.c:131: unknown field `fops' specified in initializer
drivers/char/tpm/tpm_atmel.c:131: warning: missing braces around initializer


Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>


 drivers/char/tpm/tpm_atmel.c |    2 +-
 1 files changed, 1 insertion(+), 1 deletion(-)


diff -Nru a/drivers/char/tpm/tpm_atmel.c b/drivers/char/tpm/tpm_atmel.c
--- a/drivers/char/tpm/tpm_atmel.c	2005-03-09 16:40:05 -08:00
+++ b/drivers/char/tpm/tpm_atmel.c	2005-03-09 16:40:05 -08:00
@@ -128,7 +128,7 @@
 	.req_complete_mask = ATML_STATUS_BUSY | ATML_STATUS_DATA_AVAIL,
 	.req_complete_val = ATML_STATUS_DATA_AVAIL,
 	.base = TPM_ATML_BASE,
-	.miscdev.fops = &atmel_ops,
+	.miscdev = { .fops = &atmel_ops, },
 };
 
 static int __devinit tpm_atml_init(struct pci_dev *pci_dev,

[PATCH] tpm-build-fix

[Greg KH]

ChangeSet 1.2039, 2005/03/09 10:13:34-08:00, akpm@osdl.org

[PATCH] tpm-build-fix

drivers/char/tpm/tpm.c: In function `show_pcrs':
drivers/char/tpm/tpm.c:228: warning: passing arg 1 of `tpm_transmit' from incompatible pointer type
drivers/char/tpm/tpm.c:238: warning: passing arg 1 of `tpm_transmit' from incompatible pointer type


Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>


 drivers/char/tpm/tpm.c |    2 +-
 1 files changed, 1 insertion(+), 1 deletion(-)


diff -Nru a/drivers/char/tpm/tpm.c b/drivers/char/tpm/tpm.c
--- a/drivers/char/tpm/tpm.c	2005-03-09 16:39:58 -08:00
+++ b/drivers/char/tpm/tpm.c	2005-03-09 16:39:58 -08:00
@@ -219,7 +219,7 @@
 	int i, j, index, num_pcrs;
 	char *str = buf;
 
-	struct tpm_chp *chip =
+	struct tpm_chip *chip =
 	    pci_get_drvdata(container_of(dev, struct pci_dev, dev));
 	if (chip == NULL)
 		return -ENODEV;

Re: [PATCH] Add TPM hardware enablement driver

[Jeff Garzik]

Greg KH wrote:
> diff -Nru a/drivers/char/tpm/tpm.c b/drivers/char/tpm/tpm.c
> --- /dev/null	Wed Dec 31 16:00:00 196900
> +++ b/drivers/char/tpm/tpm.c	2005-03-09 16:40:26 -08:00
> @@ -0,0 +1,697 @@
> +/*
> + * Copyright (C) 2004 IBM Corporation
> + *
> + * Authors:
> + * Leendert van Doorn <leendert@watson.ibm.com>
> + * Dave Safford <safford@watson.ibm.com>
> + * Reiner Sailer <sailer@watson.ibm.com>
> + * Kylene Hall <kjhall@us.ibm.com>
> + *
> + * Maintained by: <tpmdd_devel@lists.sourceforge.net>
> + *
> + * Device driver for TCG/TCPA TPM (trusted platform module).
> + * Specifications at www.trustedcomputinggroup.org	 
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation, version 2 of the
> + * License.
> + * 
> + * Note, the TPM chip is not interrupt driven (only polling)
> + * and can have very long timeouts (minutes!). Hence the unusual
> + * calls to schedule_timeout.
> + *
> + */
> +
> +#include <linux/sched.h>
> +#include <linux/poll.h>
> +#include <linux/spinlock.h>
> +#include "tpm.h"
> +
> +#define	TPM_MINOR			224	/* officially assigned */
> +
> +#define	TPM_BUFSIZE			2048
> +
> +/* PCI configuration addresses */
> +#define	PCI_GEN_PMCON_1			0xA0
> +#define	PCI_GEN1_DEC			0xE4
> +#define	PCI_LPC_EN			0xE6
> +#define	PCI_GEN2_DEC			0xEC

enums preferred to #defines, as these provide more type information, and 
are more visible in a debugger.


> +static LIST_HEAD(tpm_chip_list);
> +static spinlock_t driver_lock = SPIN_LOCK_UNLOCKED;
> +static int dev_mask[32];

don't use '32', create a constant and use that constant instead.


> +static void user_reader_timeout(unsigned long ptr)
> +{
> +	struct tpm_chip *chip = (struct tpm_chip *) ptr;
> +
> +	down(&chip->buffer_mutex);
> +	atomic_set(&chip->data_pending, 0);
> +	memset(chip->data_buffer, 0, TPM_BUFSIZE);
> +	up(&chip->buffer_mutex);
> +}
> +
> +void tpm_time_expired(unsigned long ptr)
> +{
> +	int *exp = (int *) ptr;
> +	*exp = 1;
> +}
> +
> +EXPORT_SYMBOL_GPL(tpm_time_expired);
> +
> +/*
> + * Initialize the LPC bus and enable the TPM ports
> + */
> +int tpm_lpc_bus_init(struct pci_dev *pci_dev, u16 base)
> +{
> +	u32 lpcenable, tmp;
> +	int is_lpcm = 0;
> +
> +	switch (pci_dev->vendor) {
> +	case PCI_VENDOR_ID_INTEL:
> +		switch (pci_dev->device) {
> +		case PCI_DEVICE_ID_INTEL_82801CA_12:
> +		case PCI_DEVICE_ID_INTEL_82801DB_12:
> +			is_lpcm = 1;
> +			break;
> +		}
> +		/* init ICH (enable LPC) */
> +		pci_read_config_dword(pci_dev, PCI_GEN1_DEC, &lpcenable);
> +		lpcenable |= 0x20000000;
> +		pci_write_config_dword(pci_dev, PCI_GEN1_DEC, lpcenable);
> +
> +		if (is_lpcm) {
> +			pci_read_config_dword(pci_dev, PCI_GEN1_DEC,
> +					      &lpcenable);
> +			if ((lpcenable & 0x20000000) == 0) {
> +				dev_err(&pci_dev->dev,
> +					"cannot enable LPC\n");
> +				return -ENODEV;
> +			}
> +		}
> +
> +		/* initialize TPM registers */
> +		pci_read_config_dword(pci_dev, PCI_GEN2_DEC, &tmp);
> +
> +		if (!is_lpcm)
> +			tmp = (tmp & 0xFFFF0000) | (base & 0xFFF0);
> +		else
> +			tmp =
> +			    (tmp & 0xFFFF0000) | (base & 0xFFF0) |
> +			    0x00000001;
> +
> +		pci_write_config_dword(pci_dev, PCI_GEN2_DEC, tmp);
> +
> +		if (is_lpcm) {
> +			pci_read_config_dword(pci_dev, PCI_GEN_PMCON_1,
> +					      &tmp);
> +			tmp |= 0x00000004;	/* enable CLKRUN */
> +			pci_write_config_dword(pci_dev, PCI_GEN_PMCON_1,
> +					       tmp);
> +		}
> +		tpm_write_index(0x0D, 0x55);	/* unlock 4F */
> +		tpm_write_index(0x0A, 0x00);	/* int disable */
> +		tpm_write_index(0x08, base);	/* base addr lo */
> +		tpm_write_index(0x09, (base & 0xFF00) >> 8);	/* base addr hi */
> +		tpm_write_index(0x0D, 0xAA);	/* lock 4F */

please define symbol names for the TPM registers


> +		break;
> +	case PCI_VENDOR_ID_AMD:
> +		/* nothing yet */
> +		break;
> +	}
> +
> +	return 0;
> +}
> +
> +EXPORT_SYMBOL_GPL(tpm_lpc_bus_init);
> +
> +/*
> + * Internal kernel interface to transmit TPM commands
> + */
> +static ssize_t tpm_transmit(struct tpm_chip *chip, const char *buf,
> +			    size_t bufsiz)
> +{
> +	ssize_t len;
> +	u32 count;
> +	__be32 *native_size;
> +
> +	native_size = (__force __be32 *) (buf + 2);
> +	count = be32_to_cpu(*native_size);
> +
> +	if (count == 0)
> +		return -ENODATA;
> +	if (count > bufsiz) {
> +		dev_err(&chip->pci_dev->dev,
> +			"invalid count value %x %x \n", count, bufsiz);
> +		return -E2BIG;
> +	}
> +
> +	down(&chip->tpm_mutex);
> +
> +	if ((len = chip->vendor->send(chip, (u8 *) buf, count)) < 0) {
> +		dev_err(&chip->pci_dev->dev,
> +			"tpm_transmit: tpm_send: error %d\n", len);
> +		return len;
> +	}
> +
> +	down(&chip->timer_manipulation_mutex);
> +	chip->time_expired = 0;
> +	init_timer(&chip->device_timer);
> +	chip->device_timer.function = tpm_time_expired;
> +	chip->device_timer.expires = jiffies + 2 * 60 * HZ;
> +	chip->device_timer.data = (unsigned long) &chip->time_expired;
> +	add_timer(&chip->device_timer);

very wrong.  you init_timer() when you initialize 'chip'... once.  then 
during the device lifetime you add/mod/del the timer.

calling init_timer() could lead to corruption of state.


> +	up(&chip->timer_manipulation_mutex);
> +
> +	do {
> +		u8 status = inb(chip->vendor->base + 1);
> +		if ((status & chip->vendor->req_complete_mask) ==
> +		    chip->vendor->req_complete_val) {
> +			down(&chip->timer_manipulation_mutex);
> +			del_singleshot_timer_sync(&chip->device_timer);
> +			up(&chip->timer_manipulation_mutex);
> +			goto out_recv;
> +		}
> +		set_current_state(TASK_UNINTERRUPTIBLE);
> +		schedule_timeout(TPM_TIMEOUT);
> +		rmb();
> +	} while (!chip->time_expired);
> +
> +
> +	chip->vendor->cancel(chip);
> +	dev_err(&chip->pci_dev->dev, "Time expired\n");
> +	up(&chip->tpm_mutex);
> +	return -EIO;
> +
> +out_recv:
> +	len = chip->vendor->recv(chip, (u8 *) buf, bufsiz);
> +	if (len < 0)
> +		dev_err(&chip->pci_dev->dev,
> +			"tpm_transmit: tpm_recv: error %d\n", len);
> +	up(&chip->tpm_mutex);
> +	return len;
> +}
> +
> +#define TPM_DIGEST_SIZE 20
> +#define CAP_PCR_RESULT_SIZE 18
> +static u8 cap_pcr[] = {
> +	0, 193,			/* TPM_TAG_RQU_COMMAND */
> +	0, 0, 0, 22,		/* length */
> +	0, 0, 0, 101,		/* TPM_ORD_GetCapability */
> +	0, 0, 0, 5,
> +	0, 0, 0, 4,
> +	0, 0, 1, 1
> +};

const


> +#define READ_PCR_RESULT_SIZE 30
> +static u8 pcrread[] = {
> +	0, 193,			/* TPM_TAG_RQU_COMMAND */
> +	0, 0, 0, 14,		/* length */
> +	0, 0, 0, 21,		/* TPM_ORD_PcrRead */
> +	0, 0, 0, 0		/* PCR index */
> +};

const


> +static ssize_t show_pcrs(struct device *dev, char *buf)
> +{
> +	u8 data[READ_PCR_RESULT_SIZE];
> +	ssize_t len;
> +	int i, j, index, num_pcrs;
> +	char *str = buf;
> +
> +	struct tpm_chp *chip =
> +	    pci_get_drvdata(container_of(dev, struct pci_dev, dev));

use to_pci_dev()


> +	if (chip == NULL)
> +		return -ENODEV;
> +
> +	memcpy(data, cap_pcr, sizeof(cap_pcr));
> +	if ((len = tpm_transmit(chip, data, sizeof(data)))
> +	    < CAP_PCR_RESULT_SIZE)
> +		return len;
> +
> +	num_pcrs = be32_to_cpu(*((__force __be32 *) (data + 14)));
> +
> +	for (i = 0; i < num_pcrs; i++) {
> +		memcpy(data, pcrread, sizeof(pcrread));
> +		index = cpu_to_be32(i);
> +		memcpy(data + 10, &index, 4);
> +		if ((len = tpm_transmit(chip, data, sizeof(data)))
> +		    < READ_PCR_RESULT_SIZE)
> +			return len;
> +		str += sprintf(str, "PCR-%02d: ", i);
> +		for (j = 0; j < TPM_DIGEST_SIZE; j++)
> +			str += sprintf(str, "%02X ", *(data + 10 + j));
> +		str += sprintf(str, "\n");
> +	}
> +	return str - buf;
> +}
> +
> +static DEVICE_ATTR(pcrs, S_IRUGO, show_pcrs, NULL);
> +
> +#define  READ_PUBEK_RESULT_SIZE 314
> +static u8 readpubek[] = {
> +	0, 193,			/* TPM_TAG_RQU_COMMAND */
> +	0, 0, 0, 30,		/* length */
> +	0, 0, 0, 124,		/* TPM_ORD_ReadPubek */
> +};
> +
> +static ssize_t show_pubek(struct device *dev, char *buf)
> +{
> +	u8 data[READ_PUBEK_RESULT_SIZE];

massive obj on stack


> +	ssize_t len;
> +	__be32 *native_val;
> +	int i;
> +	char *str = buf;
> +
> +	struct tpm_chip *chip =
> +	    pci_get_drvdata(container_of(dev, struct pci_dev, dev));

to_pci_dev()


> +	if (chip == NULL)
> +		return -ENODEV;
> +
> +	memcpy(data, readpubek, sizeof(readpubek));
> +	memset(data + sizeof(readpubek), 0, 20);	/* zero nonce */
> +
> +	if ((len = tpm_transmit(chip, data, sizeof(data))) <
> +	    READ_PUBEK_RESULT_SIZE)
> +		return len;
> +
> +	/* 
> +	   ignore header 10 bytes
> +	   algorithm 32 bits (1 == RSA )
> +	   encscheme 16 bits
> +	   sigscheme 16 bits
> +	   parameters (RSA 12->bytes: keybit, #primes, expbit)  
> +	   keylenbytes 32 bits
> +	   256 byte modulus
> +	   ignore checksum 20 bytes
> +	 */
> +
> +	native_val = (__force __be32 *) (data + 34);
> +
> +	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(*native_val)
> +	    );
> +
> +	for (i = 0; i < 256; i++) {
> +		str += sprintf(str, "%02X ", data[i + 39]);
> +		if ((i + 1) % 16 == 0)
> +			str += sprintf(str, "\n");
> +	}
> +	return str - buf;
> +}
> +
> +static DEVICE_ATTR(pubek, S_IRUGO, show_pubek, NULL);
> +
> +#define CAP_VER_RESULT_SIZE 18
> +static u8 cap_version[] = {
> +	0, 193,			/* TPM_TAG_RQU_COMMAND */
> +	0, 0, 0, 18,		/* length */
> +	0, 0, 0, 101,		/* TPM_ORD_GetCapability */
> +	0, 0, 0, 6,
> +	0, 0, 0, 0
> +};

const


> +#define CAP_MANUFACTURER_RESULT_SIZE 18
> +static u8 cap_manufacturer[] = {
> +	0, 193,			/* TPM_TAG_RQU_COMMAND */
> +	0, 0, 0, 22,		/* length */
> +	0, 0, 0, 101,		/* TPM_ORD_GetCapability */
> +	0, 0, 0, 5,
> +	0, 0, 0, 4,
> +	0, 0, 1, 3
> +};

const


> +static ssize_t show_caps(struct device *dev, char *buf)
> +{
> +	u8 data[READ_PUBEK_RESULT_SIZE];

massive obj on stack


> +	ssize_t len;
> +	char *str = buf;
> +
> +	struct tpm_chip *chip =
> +	    pci_get_drvdata(container_of(dev, struct pci_dev, dev));

to_pci_dev()


> +	if (chip == NULL)
> +		return -ENODEV;
> +
> +	memcpy(data, cap_manufacturer, sizeof(cap_manufacturer));
> +
> +	if ((len = tpm_transmit(chip, data, sizeof(data))) <
> +	    CAP_MANUFACTURER_RESULT_SIZE)
> +		return len;
> +
> +	str += sprintf(str, "Manufacturer: 0x%x\n",
> +		       be32_to_cpu(*(data + 14)));
> +
> +	memcpy(data, cap_version, sizeof(cap_version));
> +
> +	if ((len = tpm_transmit(chip, data, sizeof(data))) <
> +	    CAP_VER_RESULT_SIZE)
> +		return len;
> +
> +	str +=
> +	    sprintf(str, "TCG version: %d.%d\nFirmware version: %d.%d\n",
> +		    (int) data[14], (int) data[15], (int) data[16],
> +		    (int) data[17]);
> +
> +	return str - buf;
> +}
> +
> +static DEVICE_ATTR(caps, S_IRUGO, show_caps, NULL);
> +
> +/*
> + * Device file system interface to the TPM
> + */
> +int tpm_open(struct inode *inode, struct file *file)
> +{
> +	int rc = 0, minor = iminor(inode);
> +	struct tpm_chip *chip = NULL, *pos;
> +
> +	spin_lock(&driver_lock);
> +
> +	list_for_each_entry(pos, &tpm_chip_list, list) {
> +		if (pos->vendor->miscdev.minor == minor) {
> +			chip = pos;
> +			break;
> +		}
> +	}
> +
> +	if (chip == NULL) {
> +		rc = -ENODEV;
> +		goto err_out;
> +	}
> +
> +	if (chip->num_opens) {
> +		dev_dbg(&chip->pci_dev->dev,
> +			"Another process owns this TPM\n");
> +		rc = -EBUSY;
> +		goto err_out;
> +	}
> +
> +	chip->num_opens++;
> +	pci_dev_get(chip->pci_dev);
> +
> +	spin_unlock(&driver_lock);
> +
> +	chip->data_buffer = kmalloc(TPM_BUFSIZE * sizeof(u8), GFP_KERNEL);
> +	if (chip->data_buffer == NULL) {
> +		chip->num_opens--;
> +		pci_dev_put(chip->pci_dev);
> +		return -ENOMEM;
> +	}

what is the purpose of this pci_dev_get/put?  attempting to prevent 
hotplug or something?


> +	atomic_set(&chip->data_pending, 0);
> +
> +	file->private_data = chip;
> +	return 0;
> +
> +err_out:
> +	spin_unlock(&driver_lock);
> +	return rc;
> +}
> +
> +EXPORT_SYMBOL_GPL(tpm_open);
> +
> +int tpm_release(struct inode *inode, struct file *file)
> +{
> +	struct tpm_chip *chip = file->private_data;
> +
> +	spin_lock(&driver_lock);
> +	chip->num_opens--;
> +	down(&chip->timer_manipulation_mutex);
> +	if (timer_pending(&chip->user_read_timer))
> +		del_singleshot_timer_sync(&chip->user_read_timer);
> +	else if (timer_pending(&chip->device_timer))
> +		del_singleshot_timer_sync(&chip->device_timer);
> +	up(&chip->timer_manipulation_mutex);
> +	kfree(chip->data_buffer);
> +	atomic_set(&chip->data_pending, 0);
> +
> +	pci_dev_put(chip->pci_dev);
> +	file->private_data = NULL;
> +	spin_unlock(&driver_lock);
> +	return 0;
> +}
> +
> +EXPORT_SYMBOL_GPL(tpm_release);
> +
> +ssize_t tpm_write(struct file * file, const char __user * buf,
> +		  size_t size, loff_t * off)
> +{
> +	struct tpm_chip *chip = file->private_data;
> +	int in_size = size, out_size;
> +
> +	/* cannot perform a write until the read has cleared
> +	   either via tpm_read or a user_read_timer timeout */
> +	while (atomic_read(&chip->data_pending) != 0) {
> +		set_current_state(TASK_UNINTERRUPTIBLE);
> +		schedule_timeout(TPM_TIMEOUT);

use msleep()


> +	}
> +
> +	down(&chip->buffer_mutex);
> +
> +	if (in_size > TPM_BUFSIZE)
> +		in_size = TPM_BUFSIZE;
> +
> +	if (copy_from_user
> +	    (chip->data_buffer, (void __user *) buf, in_size)) {
> +		up(&chip->buffer_mutex);
> +		return -EFAULT;
> +	}
> +
> +	/* atomic tpm command send and result receive */
> +	out_size = tpm_transmit(chip, chip->data_buffer, TPM_BUFSIZE);

major bug?  in_size may be smaller than TPM_BUFSIZE


> +	atomic_set(&chip->data_pending, out_size);
> +	up(&chip->buffer_mutex);
> +
> +	/* Set a timeout by which the reader must come claim the result */
> +	down(&chip->timer_manipulation_mutex);
> +	init_timer(&chip->user_read_timer);
> +	chip->user_read_timer.function = user_reader_timeout;
> +	chip->user_read_timer.data = (unsigned long) chip;
> +	chip->user_read_timer.expires = jiffies + (60 * HZ);
> +	add_timer(&chip->user_read_timer);

again, don't repeatedly init_timer()


> +	up(&chip->timer_manipulation_mutex);
> +
> +	return in_size;
> +}
> +
> +EXPORT_SYMBOL_GPL(tpm_write);
> +
> +ssize_t tpm_read(struct file * file, char __user * buf,
> +		 size_t size, loff_t * off)
> +{
> +	struct tpm_chip *chip = file->private_data;
> +	int ret_size = -ENODATA;
> +
> +	if (atomic_read(&chip->data_pending) != 0) {	/* Result available */
> +		down(&chip->timer_manipulation_mutex);
> +		del_singleshot_timer_sync(&chip->user_read_timer);
> +		up(&chip->timer_manipulation_mutex);
> +
> +		down(&chip->buffer_mutex);
> +
> +		ret_size = atomic_read(&chip->data_pending);
> +		atomic_set(&chip->data_pending, 0);
> +
> +		if (ret_size == 0)	/* timeout just occurred */
> +			ret_size = -ETIME;
> +		else if (ret_size > 0) {	/* relay data */
> +			if (size < ret_size)
> +				ret_size = size;
> +
> +			if (copy_to_user((void __user *) buf,
> +					 chip->data_buffer, ret_size)) {
> +				ret_size = -EFAULT;
> +			}
> +		}
> +		up(&chip->buffer_mutex);
> +	}
> +
> +	return ret_size;

POSIX violation -- when there is no data available, returning a 
non-standard error is silly


> +}
> +
> +EXPORT_SYMBOL_GPL(tpm_read);
> +
> +void __devexit tpm_remove(struct pci_dev *pci_dev)
> +{
> +	struct tpm_chip *chip = pci_get_drvdata(pci_dev);
> +
> +	if (chip == NULL) {
> +		dev_err(&pci_dev->dev, "No device data found\n");
> +		return;
> +	}
> +
> +	spin_lock(&driver_lock);
> +
> +	list_del(&chip->list);
> +
> +	pci_set_drvdata(pci_dev, NULL);
> +	misc_deregister(&chip->vendor->miscdev);
> +
> +	device_remove_file(&pci_dev->dev, &dev_attr_pubek);
> +	device_remove_file(&pci_dev->dev, &dev_attr_pcrs);
> +	device_remove_file(&pci_dev->dev, &dev_attr_caps);
> +
> +	spin_unlock(&driver_lock);
> +
> +	pci_disable_device(pci_dev);
> +
> +	dev_mask[chip->dev_num / 32] &= !(1 << (chip->dev_num % 32));
> +
> +	kfree(chip);
> +
> +	pci_dev_put(pci_dev);
> +}

similar comment as before...  I don't see the need for pci_dev_put?


> +EXPORT_SYMBOL_GPL(tpm_remove);
> +
> +static u8 savestate[] = {
> +	0, 193,			/* TPM_TAG_RQU_COMMAND */
> +	0, 0, 0, 10,		/* blob length (in bytes) */
> +	0, 0, 0, 152		/* TPM_ORD_SaveState */
> +};

const


> +/*
> + * We are about to suspend. Save the TPM state
> + * so that it can be restored.
> + */
> +int tpm_pm_suspend(struct pci_dev *pci_dev, u32 pm_state)
> +{
> +	struct tpm_chip *chip = pci_get_drvdata(pci_dev);
> +	if (chip == NULL)
> +		return -ENODEV;
> +
> +	tpm_transmit(chip, savestate, sizeof(savestate));
> +	return 0;
> +}
> +
> +EXPORT_SYMBOL_GPL(tpm_pm_suspend);
> +
> +/*
> + * Resume from a power safe. The BIOS already restored
> + * the TPM state.
> + */
> +int tpm_pm_resume(struct pci_dev *pci_dev)
> +{
> +	struct tpm_chip *chip = pci_get_drvdata(pci_dev);
> +	if (chip == NULL)
> +		return -ENODEV;
> +
> +	spin_lock(&driver_lock);
> +	tpm_lpc_bus_init(pci_dev, chip->vendor->base);
> +	spin_unlock(&driver_lock);
> +
> +	return 0;
> +}
> +
> +EXPORT_SYMBOL_GPL(tpm_pm_resume);
> +
> +/*
> + * Called from tpm_<specific>.c probe function only for devices 
> + * the driver has determined it should claim.  Prior to calling
> + * this function the specific probe function has called pci_enable_device
> + * upon errant exit from this function specific probe function should call
> + * pci_disable_device
> + */
> +int tpm_register_hardware(struct pci_dev *pci_dev,
> +			  struct tpm_vendor_specific *entry)
> +{
> +	char devname[7];
> +	struct tpm_chip *chip;
> +	int i, j;
> +
> +	/* Driver specific per-device data */
> +	chip = kmalloc(sizeof(*chip), GFP_KERNEL);
> +	if (chip == NULL)
> +		return -ENOMEM;
> +
> +	memset(chip, 0, sizeof(struct tpm_chip));
> +
> +	init_MUTEX(&chip->buffer_mutex);
> +	init_MUTEX(&chip->tpm_mutex);
> +	init_MUTEX(&chip->timer_manipulation_mutex);
> +	INIT_LIST_HEAD(&chip->list);

timer init should be here


> +	chip->vendor = entry;
> +
> +	chip->dev_num = -1;
> +
> +	for (i = 0; i < 32; i++)
> +		for (j = 0; j < 8; j++)
> +			if ((dev_mask[i] & (1 << j)) == 0) {
> +				chip->dev_num = i * 32 + j;
> +				dev_mask[i] |= 1 << j;
> +				goto dev_num_search_complete;
> +			}
> +
> +dev_num_search_complete:
> +	if (chip->dev_num < 0) {
> +		dev_err(&pci_dev->dev,
> +			"No available tpm device numbers\n");
> +		kfree(chip);
> +		return -ENODEV;
> +	} else if (chip->dev_num == 0)
> +		chip->vendor->miscdev.minor = TPM_MINOR;
> +	else
> +		chip->vendor->miscdev.minor = MISC_DYNAMIC_MINOR;
> +
> +	snprintf(devname, sizeof(devname), "%s%d", "tpm", chip->dev_num);
> +	chip->vendor->miscdev.name = devname;
> +
> +	chip->vendor->miscdev.dev = &(pci_dev->dev);
> +	chip->pci_dev = pci_dev_get(pci_dev);
> +
> +	spin_lock(&driver_lock);
> +
> +	if (misc_register(&chip->vendor->miscdev)) {
> +		dev_err(&chip->pci_dev->dev,
> +			"unable to misc_register %s, minor %d\n",
> +			chip->vendor->miscdev.name,
> +			chip->vendor->miscdev.minor);
> +		pci_dev_put(pci_dev);
> +		spin_unlock(&driver_lock);
> +		kfree(chip);
> +		dev_mask[i] &= !(1 << j);
> +		return -ENODEV;
> +	}
> +
> +	pci_set_drvdata(pci_dev, chip);
> +
> +	list_add(&chip->list, &tpm_chip_list);
> +
> +	device_create_file(&pci_dev->dev, &dev_attr_pubek);
> +	device_create_file(&pci_dev->dev, &dev_attr_pcrs);
> +	device_create_file(&pci_dev->dev, &dev_attr_caps);
> +
> +	spin_unlock(&driver_lock);
> +	return 0;
> +}
> +
> +EXPORT_SYMBOL_GPL(tpm_register_hardware);
> +
> +static int __init init_tpm(void)
> +{
> +	return 0;
> +}
> +
> +static void __exit cleanup_tpm(void)
> +{
> +
> +}
> +
> +module_init(init_tpm);
> +module_exit(cleanup_tpm);

why?  just delete these, I would say.


> +MODULE_AUTHOR("Leendert van Doorn (leendert@watson.ibm.com)");
> +MODULE_DESCRIPTION("TPM Driver");
> +MODULE_VERSION("2.0");
> +MODULE_LICENSE("GPL");
> diff -Nru a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> --- /dev/null	Wed Dec 31 16:00:00 196900
> +++ b/drivers/char/tpm/tpm.h	2005-03-09 16:40:26 -08:00
> @@ -0,0 +1,92 @@
> +/*
> + * Copyright (C) 2004 IBM Corporation
> + *
> + * Authors:
> + * Leendert van Doorn <leendert@watson.ibm.com>
> + * Dave Safford <safford@watson.ibm.com>
> + * Reiner Sailer <sailer@watson.ibm.com>
> + * Kylene Hall <kjhall@us.ibm.com>
> + *
> + * Maintained by: <tpmdd_devel@lists.sourceforge.net>
> + *
> + * Device driver for TCG/TCPA TPM (trusted platform module).
> + * Specifications at www.trustedcomputinggroup.org	 
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation, version 2 of the
> + * License.
> + * 
> + */
> +#include <linux/module.h>
> +#include <linux/version.h>
> +#include <linux/pci.h>
> +#include <linux/delay.h>
> +#include <linux/miscdevice.h>
> +
> +#define TPM_TIMEOUT msecs_to_jiffies(5)
> +
> +/* TPM addresses */
> +#define	TPM_ADDR			0x4E
> +#define	TPM_DATA			0x4F

enum preferred to #define


> +struct tpm_chip;
> +
> +struct tpm_vendor_specific {
> +	u8 req_complete_mask;
> +	u8 req_complete_val;
> +	u16 base;		/* TPM base address */
> +
> +	int (*recv) (struct tpm_chip *, u8 *, size_t);
> +	int (*send) (struct tpm_chip *, u8 *, size_t);
> +	void (*cancel) (struct tpm_chip *);
> +	struct miscdevice miscdev;
> +};
> +
> +struct tpm_chip {
> +	struct pci_dev *pci_dev;	/* PCI device stuff */
> +
> +	int dev_num;		/* /dev/tpm# */
> +	int num_opens;		/* only one allowed */
> +	int time_expired;
> +
> +	/* Data passed to and from the tpm via the read/write calls */
> +	u8 *data_buffer;
> +	atomic_t data_pending;
> +	struct semaphore buffer_mutex;
> +
> +	struct timer_list user_read_timer;	/* user needs to claim result */
> +	struct semaphore tpm_mutex;	/* tpm is processing */
> +	struct timer_list device_timer;	/* tpm is processing */
> +	struct semaphore timer_manipulation_mutex;
> +
> +	struct tpm_vendor_specific *vendor;
> +
> +	struct list_head list;
> +};
> +
> +static inline int tpm_read_index(int index)
> +{
> +	outb(index, TPM_ADDR);
> +	return inb(TPM_DATA) & 0xFF;
> +}
> +
> +static inline void tpm_write_index(int index, int value)
> +{
> +	outb(index, TPM_ADDR);
> +	outb(value & 0xFF, TPM_DATA);
> +}
> +
> +extern void tpm_time_expired(unsigned long);
> +extern int tpm_lpc_bus_init(struct pci_dev *, u16);
> +
> +extern int tpm_register_hardware(struct pci_dev *,
> +				 struct tpm_vendor_specific *);
> +extern int tpm_open(struct inode *, struct file *);
> +extern int tpm_release(struct inode *, struct file *);
> +extern ssize_t tpm_write(struct file *, const char __user *, size_t,
> +			 loff_t *);
> +extern ssize_t tpm_read(struct file *, char __user *, size_t, loff_t *);
> +extern void __devexit tpm_remove(struct pci_dev *);
> +extern int tpm_pm_suspend(struct pci_dev *, u32);
> +extern int tpm_pm_resume(struct pci_dev *);
> diff -Nru a/drivers/char/tpm/tpm_atmel.c b/drivers/char/tpm/tpm_atmel.c
> --- /dev/null	Wed Dec 31 16:00:00 196900
> +++ b/drivers/char/tpm/tpm_atmel.c	2005-03-09 16:40:26 -08:00
> @@ -0,0 +1,216 @@
> +/*
> + * Copyright (C) 2004 IBM Corporation
> + *
> + * Authors:
> + * Leendert van Doorn <leendert@watson.ibm.com>
> + * Dave Safford <safford@watson.ibm.com>
> + * Reiner Sailer <sailer@watson.ibm.com>
> + * Kylene Hall <kjhall@us.ibm.com>
> + *
> + * Maintained by: <tpmdd_devel@lists.sourceforge.net>
> + *
> + * Device driver for TCG/TCPA TPM (trusted platform module).
> + * Specifications at www.trustedcomputinggroup.org	 
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation, version 2 of the
> + * License.
> + * 
> + */
> +
> +#include "tpm.h"
> +
> +/* Atmel definitions */
> +#define	TPM_ATML_BASE			0x400
> +
> +/* write status bits */
> +#define	ATML_STATUS_ABORT		0x01
> +#define	ATML_STATUS_LASTBYTE		0x04
> +
> +/* read status bits */
> +#define	ATML_STATUS_BUSY		0x01
> +#define	ATML_STATUS_DATA_AVAIL		0x02
> +#define	ATML_STATUS_REWRITE		0x04

enum preferred


> +static int tpm_atml_recv(struct tpm_chip *chip, u8 * buf, size_t count)
> +{
> +	u8 status, *hdr = buf;
> +	u32 size;
> +	int i;
> +	__be32 *native_size;
> +
> +	/* start reading header */
> +	if (count < 6)
> +		return -EIO;
> +
> +	for (i = 0; i < 6; i++) {
> +		status = inb(chip->vendor->base + 1);
> +		if ((status & ATML_STATUS_DATA_AVAIL) == 0) {
> +			dev_err(&chip->pci_dev->dev,
> +				"error reading header\n");
> +			return -EIO;
> +		}
> +		*buf++ = inb(chip->vendor->base);
> +	}
> +
> +	/* size of the data received */
> +	native_size = (__force __be32 *) (hdr + 2);
> +	size = be32_to_cpu(*native_size);
> +
> +	if (count < size) {
> +		dev_err(&chip->pci_dev->dev,
> +			"Recv size(%d) less than available space\n", size);
> +		for (; i < size; i++) {	/* clear the waiting data anyway */
> +			status = inb(chip->vendor->base + 1);
> +			if ((status & ATML_STATUS_DATA_AVAIL) == 0) {
> +				dev_err(&chip->pci_dev->dev,
> +					"error reading data\n");
> +				return -EIO;
> +			}
> +		}
> +		return -EIO;
> +	}

are you REALLY sure you want to eat data?


> +	/* read all the data available */
> +	for (; i < size; i++) {
> +		status = inb(chip->vendor->base + 1);
> +		if ((status & ATML_STATUS_DATA_AVAIL) == 0) {
> +			dev_err(&chip->pci_dev->dev,
> +				"error reading data\n");
> +			return -EIO;
> +		}
> +		*buf++ = inb(chip->vendor->base);
> +	}
> +
> +	/* make sure data available is gone */
> +	status = inb(chip->vendor->base + 1);
> +	if (status & ATML_STATUS_DATA_AVAIL) {
> +		dev_err(&chip->pci_dev->dev, "data available is stuck\n");
> +		return -EIO;
> +	}
> +
> +	return size;
> +}
> +
> +static int tpm_atml_send(struct tpm_chip *chip, u8 * buf, size_t count)
> +{
> +	int i;
> +
> +	dev_dbg(&chip->pci_dev->dev, "tpm_atml_send: ");
> +	for (i = 0; i < count; i++) {
> +		dev_dbg(&chip->pci_dev->dev, "0x%x(%d) ", buf[i], buf[i]);
> +		outb(buf[i], chip->vendor->base);
> +	}
> +
> +	return count;
> +}
> +
> +static void tpm_atml_cancel(struct tpm_chip *chip)
> +{
> +	outb(ATML_STATUS_ABORT, chip->vendor->base + 1);
> +}
> +
> +static struct file_operations atmel_ops = {
> +	.owner = THIS_MODULE,
> +	.llseek = no_llseek,
> +	.open = tpm_open,
> +	.read = tpm_read,
> +	.write = tpm_write,
> +	.release = tpm_release,
> +};
> +
> +static struct tpm_vendor_specific tpm_atmel = {
> +	.recv = tpm_atml_recv,
> +	.send = tpm_atml_send,
> +	.cancel = tpm_atml_cancel,
> +	.req_complete_mask = ATML_STATUS_BUSY | ATML_STATUS_DATA_AVAIL,
> +	.req_complete_val = ATML_STATUS_DATA_AVAIL,
> +	.base = TPM_ATML_BASE,
> +	.miscdev.fops = &atmel_ops,
> +};
> +
> +static int __devinit tpm_atml_init(struct pci_dev *pci_dev,
> +				   const struct pci_device_id *pci_id)
> +{
> +	u8 version[4];
> +	int rc = 0;
> +
> +	if (pci_enable_device(pci_dev))
> +		return -EIO;
> +
> +	if (tpm_lpc_bus_init(pci_dev, TPM_ATML_BASE)) {
> +		rc = -ENODEV;
> +		goto out_err;
> +	}
> +
> +	/* verify that it is an Atmel part */
> +	if (tpm_read_index(4) != 'A' || tpm_read_index(5) != 'T'
> +	    || tpm_read_index(6) != 'M' || tpm_read_index(7) != 'L') {
> +		rc = -ENODEV;
> +		goto out_err;
> +	}
> +
> +	/* query chip for its version number */
> +	if ((version[0] = tpm_read_index(0x00)) != 0xFF) {
> +		version[1] = tpm_read_index(0x01);
> +		version[2] = tpm_read_index(0x02);
> +		version[3] = tpm_read_index(0x03);
> +	} else {
> +		dev_info(&pci_dev->dev, "version query failed\n");
> +		rc = -ENODEV;
> +		goto out_err;
> +	}
> +
> +	if ((rc = tpm_register_hardware(pci_dev, &tpm_atmel)) < 0)
> +		goto out_err;
> +
> +	dev_info(&pci_dev->dev,
> +		 "Atmel TPM version %d.%d.%d.%d\n", version[0], version[1],
> +		 version[2], version[3]);
> +
> +	return 0;
> +out_err:
> +	pci_disable_device(pci_dev);
> +	return rc;
> +}
> +
> +static struct pci_device_id tpm_pci_tbl[] __devinitdata = {
> +	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82801BA_0)},
> +	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82801CA_12)},
> +	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82801DB_0)},
> +	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82801DB_12)},
> +	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82801EB_0)},
> +	{PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_8111_LPC)},
> +	{0,}
> +};
> +
> +MODULE_DEVICE_TABLE(pci, tpm_pci_tbl);
> +
> +static struct pci_driver atmel_pci_driver = {
> +	.name = "tpm_atmel",
> +	.id_table = tpm_pci_tbl,
> +	.probe = tpm_atml_init,
> +	.remove = __devexit_p(tpm_remove),
> +	.suspend = tpm_pm_suspend,
> +	.resume = tpm_pm_resume,
> +};
> +
> +static int __init init_atmel(void)
> +{
> +	return pci_register_driver(&atmel_pci_driver);
> +}
> +
> +static void __exit cleanup_atmel(void)
> +{
> +	pci_unregister_driver(&atmel_pci_driver);
> +}
> +
> +module_init(init_atmel);
> +module_exit(cleanup_atmel);
> +
> +MODULE_AUTHOR("Leendert van Doorn (leendert@watson.ibm.com)");
> +MODULE_DESCRIPTION("TPM Driver");
> +MODULE_VERSION("2.0");
> +MODULE_LICENSE("GPL");
> diff -Nru a/drivers/char/tpm/tpm_nsc.c b/drivers/char/tpm/tpm_nsc.c
> --- /dev/null	Wed Dec 31 16:00:00 196900
> +++ b/drivers/char/tpm/tpm_nsc.c	2005-03-09 16:40:26 -08:00
> @@ -0,0 +1,372 @@
> +/*
> + * Copyright (C) 2004 IBM Corporation
> + *
> + * Authors:
> + * Leendert van Doorn <leendert@watson.ibm.com>
> + * Dave Safford <safford@watson.ibm.com>
> + * Reiner Sailer <sailer@watson.ibm.com>
> + * Kylene Hall <kjhall@us.ibm.com>
> + *
> + * Maintained by: <tpmdd_devel@lists.sourceforge.net>
> + *
> + * Device driver for TCG/TCPA TPM (trusted platform module).
> + * Specifications at www.trustedcomputinggroup.org	 
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation, version 2 of the
> + * License.
> + * 
> + */
> +
> +#include "tpm.h"
> +
> +/* National definitions */
> +#define	TPM_NSC_BASE			0x360
> +#define	TPM_NSC_IRQ			0x07
> +
> +#define	NSC_LDN_INDEX			0x07
> +#define	NSC_SID_INDEX			0x20
> +#define	NSC_LDC_INDEX			0x30
> +#define	NSC_DIO_INDEX			0x60
> +#define	NSC_CIO_INDEX			0x62
> +#define	NSC_IRQ_INDEX			0x70
> +#define	NSC_ITS_INDEX			0x71
> +
> +#define	NSC_STATUS			0x01
> +#define	NSC_COMMAND			0x01
> +#define	NSC_DATA			0x00
> +
> +/* status bits */
> +#define	NSC_STATUS_OBF			0x01	/* output buffer full */
> +#define	NSC_STATUS_IBF			0x02	/* input buffer full */
> +#define	NSC_STATUS_F0			0x04	/* F0 */
> +#define	NSC_STATUS_A2			0x08	/* A2 */
> +#define	NSC_STATUS_RDY			0x10	/* ready to receive command */
> +#define	NSC_STATUS_IBR			0x20	/* ready to receive data */
> +
> +/* command bits */
> +#define	NSC_COMMAND_NORMAL		0x01	/* normal mode */
> +#define	NSC_COMMAND_EOC			0x03
> +#define	NSC_COMMAND_CANCEL		0x22

enum


> +/*
> + * Wait for a certain status to appear
> + */
> +static int wait_for_stat(struct tpm_chip *chip, u8 mask, u8 val, u8 * data)
> +{
> +	int expired = 0;
> +	struct timer_list status_timer =
> +	    TIMER_INITIALIZER(tpm_time_expired, jiffies + 10 * HZ,
> +			      (unsigned long) &expired);
> +
> +	/* status immediately available check */
> +	*data = inb(chip->vendor->base + NSC_STATUS);
> +	if ((*data & mask) == val)
> +		return 0;
> +
> +	/* wait for status */
> +	add_timer(&status_timer);
> +	do {
> +		set_current_state(TASK_UNINTERRUPTIBLE);
> +		schedule_timeout(TPM_TIMEOUT);

use msleep()


> +		*data = inb(chip->vendor->base + 1);
> +		if ((*data & mask) == val) {
> +			del_singleshot_timer_sync(&status_timer);
> +			return 0;
> +		}
> +	}
> +	while (!expired);

infinite loop:  expired never cleared

was this code ever tested?


> +	return -EBUSY;
> +}
> +
> +static int nsc_wait_for_ready(struct tpm_chip *chip)
> +{
> +	int status;
> +	int expired = 0;
> +	struct timer_list status_timer =
> +	    TIMER_INITIALIZER(tpm_time_expired, jiffies + 100,
> +			      (unsigned long) &expired);
> +
> +	/* status immediately available check */
> +	status = inb(chip->vendor->base + NSC_STATUS);
> +	if (status & NSC_STATUS_OBF)
> +		status = inb(chip->vendor->base + NSC_DATA);
> +	if (status & NSC_STATUS_RDY)
> +		return 0;
> +
> +	/* wait for status */
> +	add_timer(&status_timer);
> +	do {
> +		set_current_state(TASK_UNINTERRUPTIBLE);
> +		schedule_timeout(TPM_TIMEOUT);

msleep()


> +		status = inb(chip->vendor->base + NSC_STATUS);
> +		if (status & NSC_STATUS_OBF)
> +			status = inb(chip->vendor->base + NSC_DATA);
> +		if (status & NSC_STATUS_RDY) {
> +			del_singleshot_timer_sync(&status_timer);
> +			return 0;
> +		}
> +	}
> +	while (!expired);

another infinite loop?

Re: [PATCH] Add TPM hardware enablement driver

[Kylene Jo Hall]

Thanks for the helpful comments I am working on a patch to fix your
concerns but I have a couple of questions.  

On Wed, 2005-03-09 at 22:51 -0500, Jeff Garzik wrote:
<snip>

> > +	down(&chip->timer_manipulation_mutex);
> > +	chip->time_expired = 0;
> > +	init_timer(&chip->device_timer);
> > +	chip->device_timer.function = tpm_time_expired;
> > +	chip->device_timer.expires = jiffies + 2 * 60 * HZ;
> > +	chip->device_timer.data = (unsigned long) &chip->time_expired;
> > +	add_timer(&chip->device_timer);
> 
> very wrong.  you init_timer() when you initialize 'chip'... once.  then 
> during the device lifetime you add/mod/del the timer.
> 
> calling init_timer() could lead to corruption of state.
> 
> > +	up(&chip->timer_manipulation_mutex);
When calling mod_timer and an occasional del_singleshot_timer_sync is it
necessary to protect this with a mutex like I was doing or not?


> > +	pci_dev_get(chip->pci_dev);
> > +
> > +	spin_unlock(&driver_lock);
> > +
> > +	chip->data_buffer = kmalloc(TPM_BUFSIZE * sizeof(u8), GFP_KERNEL);
> > +	if (chip->data_buffer == NULL) {
> > +		chip->num_opens--;
> > +		pci_dev_put(chip->pci_dev);
> > +		return -ENOMEM;
> > +	}
> 
> what is the purpose of this pci_dev_get/put?  attempting to prevent 
> hotplug or something?

We were doing reference counting since their is a pci_dev in the chip
structure which set to the file->private_data pointer. Not correct?

> 
> > +	}
> > +
> > +	down(&chip->buffer_mutex);
> > +
> > +	if (in_size > TPM_BUFSIZE)
> > +		in_size = TPM_BUFSIZE;
> > +
> > +	if (copy_from_user
> > +	    (chip->data_buffer, (void __user *) buf, in_size)) {
> > +		up(&chip->buffer_mutex);
> > +		return -EFAULT;
> > +	}
> > +
> > +	/* atomic tpm command send and result receive */
> > +	out_size = tpm_transmit(chip, chip->data_buffer, TPM_BUFSIZE);
> 
> major bug?  in_size may be smaller than TPM_BUFSIZE
> 
Yes in_size might be but the chip->data_buffer will always be this size since it is malloc'd in open.  The operation needs to be atomic so the tpm_transmit function is sending the command to the device and receiving the result which might be bigger than in_size.  The result is reported back to userspace from this buffer on a read.

> > +
> > +ssize_t tpm_read(struct file * file, char __user * buf,
> > +		 size_t size, loff_t * off)
> > +{
> > +	struct tpm_chip *chip = file->private_data;
> > +	int ret_size = -ENODATA;
> > +
> > +	if (atomic_read(&chip->data_pending) != 0) {	/* Result available */
> > +		down(&chip->timer_manipulation_mutex);
> > +		del_singleshot_timer_sync(&chip->user_read_timer);
> > +		up(&chip->timer_manipulation_mutex);
> > +
> > +		down(&chip->buffer_mutex);
> > +
> > +		ret_size = atomic_read(&chip->data_pending);
> > +		atomic_set(&chip->data_pending, 0);
> > +
> > +		if (ret_size == 0)	/* timeout just occurred */
> > +			ret_size = -ETIME;
> > +		else if (ret_size > 0) {	/* relay data */
> > +			if (size < ret_size)
> > +				ret_size = size;
> > +
> > +			if (copy_to_user((void __user *) buf,
> > +					 chip->data_buffer, ret_size)) {
> > +				ret_size = -EFAULT;
> > +			}
> > +		}
> > +		up(&chip->buffer_mutex);
> > +	}
> > +
> > +	return ret_size;
> 
> POSIX violation -- when there is no data available, returning a 
> non-standard error is silly
> 
So read should just return 0 if no data available?


Thanks,
Kylie

Re: [PATCH] Add TPM hardware enablement driver

[Kylene Hall]

The patch at the bottom addresses a number of the concerns raised in this 
email along with a couple of other comments which this generated regarding 
not needing __force and the need for a MAINTAINERS entry.

Signed-off-by: Kylene Hall <kjhall@us.ibm.com>

On Wed, 9 Mar 2005, Jeff Garzik wrote:

> Greg KH wrote:
<snip>
> > +#define	TPM_MINOR			224	/* officially assigned
> > */
> > +
> > +#define	TPM_BUFSIZE			2048
> > +
> > +/* PCI configuration addresses */
> > +#define	PCI_GEN_PMCON_1			0xA0
> > +#define	PCI_GEN1_DEC			0xE4
> > +#define	PCI_LPC_EN			0xE6
> > +#define	PCI_GEN2_DEC			0xEC
> 
> enums preferred to #defines, as these provide more type information, and are
> more visible in a debugger.

fixed

> > +static LIST_HEAD(tpm_chip_list);
> > +static spinlock_t driver_lock = SPIN_LOCK_UNLOCKED;
> > +static int dev_mask[32];
> 
> don't use '32', create a constant and use that constant instead.
fixed

> > +		tpm_write_index(0x0D, 0x55);	/* unlock 4F */
> > +		tpm_write_index(0x0A, 0x00);	/* int disable */
> > +		tpm_write_index(0x08, base);	/* base addr lo */
> > +		tpm_write_index(0x09, (base & 0xFF00) >> 8);	/* base addr
> > hi */
> > +		tpm_write_index(0x0D, 0xAA);	/* lock 4F */
> 
> please define symbol names for the TPM registers

fixed 


> > +	down(&chip->timer_manipulation_mutex);
> > +	chip->time_expired = 0;
> > +	init_timer(&chip->device_timer);
> > +	chip->device_timer.function = tpm_time_expired;
> > +	chip->device_timer.expires = jiffies + 2 * 60 * HZ;
> > +	chip->device_timer.data = (unsigned long) &chip->time_expired;
> > +	add_timer(&chip->device_timer);
> 
> very wrong.  you init_timer() when you initialize 'chip'... once.  then during
> the device lifetime you add/mod/del the timer.
> 
> calling init_timer() could lead to corruption of state.

fixed

> > +static u8 cap_pcr[] = {
> > +	0, 193,			/* TPM_TAG_RQU_COMMAND */
> > +	0, 0, 0, 22,		/* length */
> > +	0, 0, 0, 101,		/* TPM_ORD_GetCapability */
> > +	0, 0, 0, 5,
> > +	0, 0, 0, 4,
> > +	0, 0, 1, 1
> > +};
> 
> const

fixed


> > +static u8 pcrread[] = {
> > +	0, 193,			/* TPM_TAG_RQU_COMMAND */
> > +	0, 0, 0, 14,		/* length */
> > +	0, 0, 0, 21,		/* TPM_ORD_PcrRead */
> > +	0, 0, 0, 0		/* PCR index */
> > +};
> 
> const

fixed

> > +
> > +	struct tpm_chp *chip =
> > +	    pci_get_drvdata(container_of(dev, struct pci_dev, dev));
> 
> use to_pci_dev()

fixed

> > +#define  READ_PUBEK_RESULT_SIZE 314
> > +static u8 readpubek[] = {
> > +	0, 193,			/* TPM_TAG_RQU_COMMAND */
> > +	0, 0, 0, 30,		/* length */
> > +	0, 0, 0, 124,		/* TPM_ORD_ReadPubek */
> > +};
> > +
> > +static ssize_t show_pubek(struct device *dev, char *buf)
> > +{
> > +	u8 data[READ_PUBEK_RESULT_SIZE];
> 
> massive obj on stack

fixed

> > +	struct tpm_chip *chip =
> > +	    pci_get_drvdata(container_of(dev, struct pci_dev, dev));
> 
> to_pci_dev()

fixed

> > +static u8 cap_version[] = {
> > +	0, 193,			/* TPM_TAG_RQU_COMMAND */
> > +	0, 0, 0, 18,		/* length */
> > +	0, 0, 0, 101,		/* TPM_ORD_GetCapability */
> > +	0, 0, 0, 6,
> > +	0, 0, 0, 0
> > +};
> 
> const

fixed

> > +static u8 cap_manufacturer[] = {
> > +	0, 193,			/* TPM_TAG_RQU_COMMAND */
> > +	0, 0, 0, 22,		/* length */
> > +	0, 0, 0, 101,		/* TPM_ORD_GetCapability */
> > +	0, 0, 0, 5,
> > +	0, 0, 0, 4,
> > +	0, 0, 1, 3
> > +};
> 
> const

fixed

> 
> > +static ssize_t show_caps(struct device *dev, char *buf)
> > +{
> > +	u8 data[READ_PUBEK_RESULT_SIZE];
> 
> massive obj on stack

fixed

> > +	struct tpm_chip *chip =
> > +	    pci_get_drvdata(container_of(dev, struct pci_dev, dev));
> 
> to_pci_dev()

fixed

> > +	chip->data_buffer = kmalloc(TPM_BUFSIZE * sizeof(u8), GFP_KERNEL);
> > +	if (chip->data_buffer == NULL) {
> > +		chip->num_opens--;
> > +		pci_dev_put(chip->pci_dev);
> > +		return -ENOMEM;
> > +	}
> 
> what is the purpose of this pci_dev_get/put?  attempting to prevent hotplug or
> something?

Seems that since there is a refernce to the device in the chip structure 
and I am making the file private data pointer point to that chip structure 
this is another reference that must be accounted for. If you remove it 
with it open and attempt read or write bad things will happen.  This isn't 
really hotpluggable either as the TPM is on the motherboard.

> > +
> > +	/* cannot perform a write until the read has cleared
> > +	   either via tpm_read or a user_read_timer timeout */
> > +	while (atomic_read(&chip->data_pending) != 0) {
> > +		set_current_state(TASK_UNINTERRUPTIBLE);
> > +		schedule_timeout(TPM_TIMEOUT);
> 

> use msleep()

addressed in another patch by Nish

> > +	/* atomic tpm command send and result receive */
> > +	out_size = tpm_transmit(chip, chip->data_buffer, TPM_BUFSIZE);
> 
> major bug?  in_size may be smaller than TPM_BUFSIZE

chip->data_buffer is allocated in open and is always this size.  The 
operation needs to be atomic so the big buffer is to cover the size of a 
potentially larger result.  Only reading in_size from the user with 
copy_from_user

> > +	down(&chip->timer_manipulation_mutex);
> > +	init_timer(&chip->user_read_timer);
> > +	chip->user_read_timer.function = user_reader_timeout;
> > +	chip->user_read_timer.data = (unsigned long) chip;
> > +	chip->user_read_timer.expires = jiffies + (60 * HZ);
> > +	add_timer(&chip->user_read_timer);
> 
> again, don't repeatedly init_timer()
> 
fixed

> > +ssize_t tpm_read(struct file * file, char __user * buf,
> > +		 size_t size, loff_t * off)
> > +{
> > +	struct tpm_chip *chip = file->private_data;
> > +	int ret_size = -ENODATA;
> > +
> > +	if (atomic_read(&chip->data_pending) != 0) {	/* Result available */
> > +		down(&chip->timer_manipulation_mutex);
> > +		del_singleshot_timer_sync(&chip->user_read_timer);
> > +		up(&chip->timer_manipulation_mutex);
> > +
> > +		down(&chip->buffer_mutex);
> > +
> > +		ret_size = atomic_read(&chip->data_pending);
> > +		atomic_set(&chip->data_pending, 0);
> > +
> > +		if (ret_size == 0)	/* timeout just occurred */
> > +			ret_size = -ETIME;
> > +		else if (ret_size > 0) {	/* relay data */
> > +			if (size < ret_size)
> > +				ret_size = size;
> > +
> > +			if (copy_to_user((void __user *) buf,
> > +					 chip->data_buffer, ret_size)) {
> > +				ret_size = -EFAULT;
> > +			}
> > +		}
> > +		up(&chip->buffer_mutex);
> > +	}
> > +
> > +	return ret_size;
> 
> POSIX violation -- when there is no data available, returning a non-standard
> error is silly
> 
fixed

> > +
> > +	pci_dev_put(pci_dev);
> > +}
> 
> similar comment as before...  I don't see the need for pci_dev_put?

See justification above.

> > +static u8 savestate[] = {
> > +	0, 193,			/* TPM_TAG_RQU_COMMAND */
> > +	0, 0, 0, 10,		/* blob length (in bytes) */
> > +	0, 0, 0, 152		/* TPM_ORD_SaveState */
> > +};
> 
> const

fixed

> > +
> > +	init_MUTEX(&chip->buffer_mutex);
> > +	init_MUTEX(&chip->tpm_mutex);
> > +	init_MUTEX(&chip->timer_manipulation_mutex);
> > +	INIT_LIST_HEAD(&chip->list);
> 
> timer init should be here

fixed

> > +
> > +static int __init init_tpm(void)
> > +{
> > +	return 0;
> > +}
> > +
> > +static void __exit cleanup_tpm(void)
> > +{
> > +
> > +}
> > +
> > +module_init(init_tpm);
> > +module_exit(cleanup_tpm);
> 
> why?  just delete these, I would say.

fixed

> > +
> > +/* TPM addresses */
> > +#define	TPM_ADDR			0x4E
> > +#define	TPM_DATA			0x4F
> 
> enum preferred to #define

fixed

> > +/* Atmel definitions */
> > +#define	TPM_ATML_BASE			0x400
> > +
> > +/* write status bits */
> > +#define	ATML_STATUS_ABORT		0x01
> > +#define	ATML_STATUS_LASTBYTE		0x04
> > +
> > +/* read status bits */
> > +#define	ATML_STATUS_BUSY		0x01
> > +#define	ATML_STATUS_DATA_AVAIL		0x02
> > +#define	ATML_STATUS_REWRITE		0x04
> 
> enum preferred

fixed

> > +	if (count < size) {
> > +		dev_err(&chip->pci_dev->dev,
> > +			"Recv size(%d) less than available space\n", size);
> > +		for (; i < size; i++) {	/* clear the waiting data anyway */
> > +			status = inb(chip->vendor->base + 1);
> > +			if ((status & ATML_STATUS_DATA_AVAIL) == 0) {
> > +				dev_err(&chip->pci_dev->dev,
> > +					"error reading data\n");
> > +				return -EIO;
> > +			}
> > +		}
> > +		return -EIO;
> > +	}
> 
> are you REALLY sure you want to eat data?

if the buffer isn't big enough the driver is broken so I don't see this as 
a problem.  See the comment abover where tpm_tranmit is called.  That is 
how you get into this code.

> > +/* National definitions */
> > +#define	TPM_NSC_BASE			0x360
> > +#define	TPM_NSC_IRQ			0x07
> > +
> > +#define	NSC_LDN_INDEX			0x07
> > +#define	NSC_SID_INDEX			0x20
> > +#define	NSC_LDC_INDEX			0x30
> > +#define	NSC_DIO_INDEX			0x60
> > +#define	NSC_CIO_INDEX			0x62
> > +#define	NSC_IRQ_INDEX			0x70
> > +#define	NSC_ITS_INDEX			0x71
> > +
> > +#define	NSC_STATUS			0x01
> > +#define	NSC_COMMAND			0x01
> > +#define	NSC_DATA			0x00
> > +
> > +/* status bits */
> > +#define	NSC_STATUS_OBF			0x01	/* output buffer full
> > */
> > +#define	NSC_STATUS_IBF			0x02	/* input buffer full
> > */
> > +#define	NSC_STATUS_F0			0x04	/* F0 */
> > +#define	NSC_STATUS_A2			0x08	/* A2 */
> > +#define	NSC_STATUS_RDY			0x10	/* ready to receive
> > command */
> > +#define	NSC_STATUS_IBR			0x20	/* ready to receive
> > data */
> > +
> > +/* command bits */
> > +#define	NSC_COMMAND_NORMAL		0x01	/* normal mode */
> > +#define	NSC_COMMAND_EOC			0x03
> > +#define	NSC_COMMAND_CANCEL		0x22
> 
> enum

fixed

> > +		schedule_timeout(TPM_TIMEOUT);
> 
> use msleep()

fixed

> > +		}
> > +	}
> > +	while (!expired);
> 
> infinite loop:  expired never cleared
> 

fixed in another patch

> > +		set_current_state(TASK_UNINTERRUPTIBLE);
> > +		schedule_timeout(TPM_TIMEOUT);
> 
> msleep()

fixed in another patch


> > +	}
> > +	while (!expired);
> 
> another infinite loop?

fixed in another patch


New patch:


diff -uprN linux-2.6.11.3-orig/drivers/char/tpm/tpm_atmel.c linux-2.6.11.3/drivers/char/tpm/tpm_atmel.c
--- linux-2.6.11.3-orig/drivers/char/tpm/tpm_atmel.c	2005-03-16 19:31:32.000000000 -0600
+++ linux-2.6.11.3/drivers/char/tpm/tpm_atmel.c	2005-03-16 19:31:32.000000000 -0600
@@ -22,17 +22,21 @@
 #include "tpm.h"
 
 /* Atmel definitions */
-#define        TPM_ATML_BASE                   0x400
+enum {
+	TPM_ATML_BASE = 0x400
+};
 
 /* write status bits */
-#define        ATML_STATUS_ABORT               0x01
-#define        ATML_STATUS_LASTBYTE            0x04
-
+enum {
+	ATML_STATUS_ABORT = 0x01,
+	ATML_STATUS_LASTBYTE = 0x04
+};
 /* read status bits */
-#define        ATML_STATUS_BUSY                0x01
-#define        ATML_STATUS_DATA_AVAIL          0x02
-#define        ATML_STATUS_REWRITE             0x04
-
+enum {
+	ATML_STATUS_BUSY = 0x01,
+	ATML_STATUS_DATA_AVAIL = 0x02,
+	ATML_STATUS_REWRITE = 0x04
+};
 
 static int tpm_atml_recv(struct tpm_chip *chip, u8 * buf, size_t count)
 {
diff -uprN linux-2.6.11.3-orig/drivers/char/tpm/tpm.c linux-2.6.11.3/drivers/char/tpm/tpm.c
--- linux-2.6.11.3-orig/drivers/char/tpm/tpm.c	2005-03-16 19:31:32.000000000 -0600
+++ linux-2.6.11.3/drivers/char/tpm/tpm.c	2005-03-16 19:31:32.000000000 -0600
@@ -28,19 +28,34 @@
 #include <linux/spinlock.h>
 #include "tpm.h"
 
-#define        TPM_MINOR                       224	/* officially assigned */
-
-#define        TPM_BUFSIZE                     2048
+enum {
+	TPM_MINOR = 224,	/* officially assigned */
+	TPM_BUFSIZE = 2048,
+	TPM_NUM_DEVICES = 256,
+	TPM_NUM_MASK_ENTRIES = TPM_NUM_DEVICES / (8 * sizeof(int))
+};
 
 /* PCI configuration addresses */
-#define        PCI_GEN_PMCON_1                 0xA0
-#define        PCI_GEN1_DEC                    0xE4
-#define        PCI_LPC_EN                      0xE6
-#define        PCI_GEN2_DEC                    0xEC
+enum {
+	PCI_GEN_PMCON_1 = 0xA0,
+	PCI_GEN1_DEC = 0xE4,
+	PCI_LPC_EN = 0xE6,
+	PCI_GEN2_DEC = 0xEC
+};
+
+enum {
+	TPM_LOCK_REG = 0x0D,
+	TPM_INTERUPT_REG = 0x0A,
+	TPM_BASE_ADDR_LO = 0x08,
+	TPM_BASE_ADDR_HI = 0x09,
+	TPM_UNLOCK_VALUE = 0x55,
+	TPM_LOCK_VALUE = 0xAA,
+	TPM_DISABLE_INTERUPT_VALUE = 0x00
+};
 
 static LIST_HEAD(tpm_chip_list);
 static spinlock_t driver_lock = SPIN_LOCK_UNLOCKED;
-static int dev_mask[32];
+static int dev_mask[TPM_NUM_MASK_ENTRIES];
 
 static void user_reader_timeout(unsigned long ptr)
 {
@@ -102,11 +117,11 @@ int tpm_lpc_bus_init(struct pci_dev *pci
 			pci_write_config_dword(pci_dev, PCI_GEN_PMCON_1,
 					       tmp);
 		}
-		tpm_write_index(0x0D, 0x55);	/* unlock 4F */
-		tpm_write_index(0x0A, 0x00);	/* int disable */
-		tpm_write_index(0x08, base);	/* base addr lo */
-		tpm_write_index(0x09, (base & 0xFF00) >> 8);	/* base addr hi */
-		tpm_write_index(0x0D, 0xAA);	/* lock 4F */
+		tpm_write_index(TPM_LOCK_REG, TPM_UNLOCK_VALUE);	/* unlock 4F */
+		tpm_write_index(TPM_INTERUPT_REG, TPM_DISABLE_INTERUPT_VALUE);	/* int disable */
+		tpm_write_index(TPM_BASE_ADDR_LO, base);	/* base addr lo */
+		tpm_write_index(TPM_BASE_ADDR_HI, (base & 0xFF00) >> 8);	/* base addr hi */
+		tpm_write_index(TPM_LOCK_REG, TPM_LOCK_VALUE);	/* lock 4F */
 		break;
 	case PCI_VENDOR_ID_AMD:
 		/* nothing yet */
@@ -121,16 +136,14 @@ EXPORT_SYMBOL_GPL(tpm_lpc_bus_init);
 /*
  * Internal kernel interface to transmit TPM commands
  */
-static ssize_t tpm_transmit(struct tpm_chip *chip, const char *buf,
-			    size_t bufsiz)
+static ssize_t
+tpm_transmit(struct tpm_chip *chip, const char *buf, size_t bufsiz)
 {
 	ssize_t len;
 	u32 count;
-	__be32 *native_size;
 	unsigned long stop;
 
-	native_size = (__force __be32 *) (buf + 2);
-	count = be32_to_cpu(*native_size);
+	count = be32_to_cpu(*((__be32 *) (buf + 2)));
 
 	if (count == 0)
 		return -ENODATA;
@@ -157,7 +170,8 @@ static ssize_t tpm_transmit(struct tpm_c
 		}
 		msleep(TPM_TIMEOUT);	/* CHECK */
 		rmb();
-	} while (time_before(jiffies, stop));
+	}
+	while (time_before(jiffies, stop));
 
 
 	chip->vendor->cancel(chip);
@@ -176,7 +190,7 @@ static ssize_t tpm_transmit(struct tpm_c
 
 #define TPM_DIGEST_SIZE 20
 #define CAP_PCR_RESULT_SIZE 18
-static u8 cap_pcr[] = {
+static const u8 cap_pcr[] = {
 	0, 193,			/* TPM_TAG_RQU_COMMAND */
 	0, 0, 0, 22,		/* length */
 	0, 0, 0, 101,		/* TPM_ORD_GetCapability */
@@ -186,7 +200,7 @@ static u8 cap_pcr[] = {
 };
 
 #define READ_PCR_RESULT_SIZE 30
-static u8 pcrread[] = {
+static const u8 pcrread[] = {
 	0, 193,			/* TPM_TAG_RQU_COMMAND */
 	0, 0, 0, 14,		/* length */
 	0, 0, 0, 21,		/* TPM_ORD_PcrRead */
@@ -197,20 +211,20 @@ static ssize_t show_pcrs(struct device *
 {
 	u8 data[READ_PCR_RESULT_SIZE];
 	ssize_t len;
-	int i, j, index, num_pcrs;
+	int i, j, num_pcrs;
+	__be32 index;
 	char *str = buf;
 
-	struct tpm_chip *chip =
-	    pci_get_drvdata(container_of(dev, struct pci_dev, dev));
+	struct tpm_chip *chip = pci_get_drvdata(to_pci_dev(dev));
 	if (chip == NULL)
 		return -ENODEV;
 
 	memcpy(data, cap_pcr, sizeof(cap_pcr));
-	if ((len = tpm_transmit(chip, data, sizeof(data)))
-	    < CAP_PCR_RESULT_SIZE)
+	if ((len =
+	     tpm_transmit(chip, data, sizeof(data))) < CAP_PCR_RESULT_SIZE)
 		return len;
 
-	num_pcrs = be32_to_cpu(*((__force __be32 *) (data + 14)));
+	num_pcrs = be32_to_cpu(*((__be32 *) (data + 14)));
 
 	for (i = 0; i < num_pcrs; i++) {
 		memcpy(data, pcrread, sizeof(pcrread));
@@ -230,7 +244,7 @@ static ssize_t show_pcrs(struct device *
 static DEVICE_ATTR(pcrs, S_IRUGO, show_pcrs, NULL);
 
 #define  READ_PUBEK_RESULT_SIZE 314
-static u8 readpubek[] = {
+static const u8 readpubek[] = {
 	0, 193,			/* TPM_TAG_RQU_COMMAND */
 	0, 0, 0, 30,		/* length */
 	0, 0, 0, 124,		/* TPM_ORD_ReadPubek */
@@ -238,23 +252,27 @@ static u8 readpubek[] = {
 
 static ssize_t show_pubek(struct device *dev, char *buf)
 {
-	u8 data[READ_PUBEK_RESULT_SIZE];
+	u8 *data;
 	ssize_t len;
-	__be32 *native_val;
-	int i;
+	int i, rc;
 	char *str = buf;
 
-	struct tpm_chip *chip =
-	    pci_get_drvdata(container_of(dev, struct pci_dev, dev));
+	struct tpm_chip *chip = pci_get_drvdata(to_pci_dev(dev));
 	if (chip == NULL)
 		return -ENODEV;
 
+	data = kmalloc(READ_PUBEK_RESULT_SIZE, GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
 	memcpy(data, readpubek, sizeof(readpubek));
 	memset(data + sizeof(readpubek), 0, 20);	/* zero nonce */
 
 	if ((len = tpm_transmit(chip, data, sizeof(data))) <
-	    READ_PUBEK_RESULT_SIZE)
-		return len;
+	    READ_PUBEK_RESULT_SIZE) {
+		rc = len;
+		goto out;
+	}
 
 	/* 
 	   ignore header 10 bytes
@@ -267,8 +285,6 @@ static ssize_t show_pubek(struct device 
 	   ignore checksum 20 bytes
 	 */
 
-	native_val = (__force __be32 *) (data + 34);
-
 	str +=
 	    sprintf(str,
 		    "Algorithm: %02X %02X %02X %02X\nEncscheme: %02X %02X\n"
@@ -279,21 +295,23 @@ static ssize_t show_pubek(struct device 
 		    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(*native_val)
-	    );
+		    be32_to_cpu(*((__be32 *) (data + 32))));
 
 	for (i = 0; i < 256; i++) {
 		str += sprintf(str, "%02X ", data[i + 39]);
 		if ((i + 1) % 16 == 0)
 			str += sprintf(str, "\n");
 	}
-	return str - buf;
+	rc = str - buf;
+      out:
+	kfree(data);
+	return rc;
 }
 
 static DEVICE_ATTR(pubek, S_IRUGO, show_pubek, NULL);
 
 #define CAP_VER_RESULT_SIZE 18
-static u8 cap_version[] = {
+static const u8 cap_version[] = {
 	0, 193,			/* TPM_TAG_RQU_COMMAND */
 	0, 0, 0, 18,		/* length */
 	0, 0, 0, 101,		/* TPM_ORD_GetCapability */
@@ -302,7 +320,7 @@ static u8 cap_version[] = {
 };
 
 #define CAP_MANUFACTURER_RESULT_SIZE 18
-static u8 cap_manufacturer[] = {
+static const u8 cap_manufacturer[] = {
 	0, 193,			/* TPM_TAG_RQU_COMMAND */
 	0, 0, 0, 22,		/* length */
 	0, 0, 0, 101,		/* TPM_ORD_GetCapability */
@@ -313,12 +331,12 @@ static u8 cap_manufacturer[] = {
 
 static ssize_t show_caps(struct device *dev, char *buf)
 {
-	u8 data[READ_PUBEK_RESULT_SIZE];
+	u8 data[(CAP_VER_RESULT_SIZE > CAP_MANUFACTURER_RESULT_SIZE) ?
+		CAP_VER_RESULT_SIZE : CAP_MANUFACTURER_RESULT_SIZE];
 	ssize_t len;
 	char *str = buf;
 
-	struct tpm_chip *chip =
-	    pci_get_drvdata(container_of(dev, struct pci_dev, dev));
+	struct tpm_chip *chip = pci_get_drvdata(to_pci_dev(dev));
 	if (chip == NULL)
 		return -ENODEV;
 
@@ -329,12 +347,12 @@ static ssize_t show_caps(struct device *
 		return len;
 
 	str += sprintf(str, "Manufacturer: 0x%x\n",
-		       be32_to_cpu(*(data + 14)));
+		       be32_to_cpu(*((__be32 *) (data + 14))));
 
 	memcpy(data, cap_version, sizeof(cap_version));
 
-	if ((len = tpm_transmit(chip, data, sizeof(data))) <
-	    CAP_VER_RESULT_SIZE)
+	if ((len =
+	     tpm_transmit(chip, data, sizeof(data))) < CAP_VER_RESULT_SIZE)
 		return len;
 
 	str +=
@@ -404,30 +422,22 @@ int tpm_release(struct inode *inode, str
 {
 	struct tpm_chip *chip = file->private_data;
 
-	file->private_data = NULL;
-
 	spin_lock(&driver_lock);
+	file->private_data = NULL;
 	chip->num_opens--;
-	spin_unlock(&driver_lock);
-
-	down(&chip->timer_manipulation_mutex);
-	if (timer_pending(&chip->user_read_timer))
-		del_singleshot_timer_sync(&chip->user_read_timer);
-	else if (timer_pending(&chip->device_timer))
-		del_singleshot_timer_sync(&chip->device_timer);
-	up(&chip->timer_manipulation_mutex);
-
-	kfree(chip->data_buffer);
+	del_singleshot_timer_sync(&chip->user_read_timer);
 	atomic_set(&chip->data_pending, 0);
-
 	pci_dev_put(chip->pci_dev);
+	kfree(chip->data_buffer);
+	spin_unlock(&driver_lock);
 	return 0;
 }
 
 EXPORT_SYMBOL_GPL(tpm_release);
 
-ssize_t tpm_write(struct file * file, const char __user * buf,
-		  size_t size, loff_t * off)
+ssize_t
+tpm_write(struct file * file, const char __user * buf,
+	  size_t size, loff_t * off)
 {
 	struct tpm_chip *chip = file->private_data;
 	int in_size = size, out_size;
@@ -449,52 +459,38 @@ ssize_t tpm_write(struct file * file, co
 	}
 
 	/* atomic tpm command send and result receive */
+	dev_info(&chip->pci_dev->dev, "data_buffer size: %d\n",
+		 sizeof(*chip->data_buffer));
 	out_size = tpm_transmit(chip, chip->data_buffer, TPM_BUFSIZE);
 
 	atomic_set(&chip->data_pending, out_size);
 	up(&chip->buffer_mutex);
 
 	/* Set a timeout by which the reader must come claim the result */
-	down(&chip->timer_manipulation_mutex);
-	init_timer(&chip->user_read_timer);
-	chip->user_read_timer.function = user_reader_timeout;
-	chip->user_read_timer.data = (unsigned long) chip;
-	chip->user_read_timer.expires = jiffies + (60 * HZ);
-	add_timer(&chip->user_read_timer);
-	up(&chip->timer_manipulation_mutex);
+	mod_timer(&chip->user_read_timer, jiffies + (60 * HZ));
 
 	return in_size;
 }
 
 EXPORT_SYMBOL_GPL(tpm_write);
 
-ssize_t tpm_read(struct file * file, char __user * buf,
-		 size_t size, loff_t * off)
+ssize_t
+tpm_read(struct file * file, char __user * buf, size_t size, loff_t * off)
 {
 	struct tpm_chip *chip = file->private_data;
-	int ret_size = -ENODATA;
+	int ret_size;
 
-	if (atomic_read(&chip->data_pending) != 0) {	/* Result available */
-		down(&chip->timer_manipulation_mutex);
-		del_singleshot_timer_sync(&chip->user_read_timer);
-		up(&chip->timer_manipulation_mutex);
+	del_singleshot_timer_sync(&chip->user_read_timer);
+	ret_size = atomic_read(&chip->data_pending);
+	atomic_set(&chip->data_pending, 0);
+	if (ret_size > 0) {	/* relay data */
+		if (size < ret_size)
+			ret_size = size;
 
 		down(&chip->buffer_mutex);
-
-		ret_size = atomic_read(&chip->data_pending);
-		atomic_set(&chip->data_pending, 0);
-
-		if (ret_size == 0)	/* timeout just occurred */
-			ret_size = -ETIME;
-		else if (ret_size > 0) {	/* relay data */
-			if (size < ret_size)
-				ret_size = size;
-
-			if (copy_to_user((void __user *) buf,
-					 chip->data_buffer, ret_size)) {
-				ret_size = -EFAULT;
-			}
-		}
+		if (copy_to_user
+		    ((void __user *) buf, chip->data_buffer, ret_size))
+			ret_size = -EFAULT;
 		up(&chip->buffer_mutex);
 	}
 
@@ -516,8 +512,6 @@ void __devexit tpm_remove(struct pci_dev
 
 	list_del(&chip->list);
 
-	spin_unlock(&driver_lock);
-
 	pci_set_drvdata(pci_dev, NULL);
 	misc_deregister(&chip->vendor->miscdev);
 
@@ -525,9 +519,12 @@ void __devexit tpm_remove(struct pci_dev
 	device_remove_file(&pci_dev->dev, &dev_attr_pcrs);
 	device_remove_file(&pci_dev->dev, &dev_attr_caps);
 
+	spin_unlock(&driver_lock);
+
 	pci_disable_device(pci_dev);
 
-	dev_mask[chip->dev_num / 32] &= !(1 << (chip->dev_num % 32));
+	dev_mask[chip->dev_num / TPM_NUM_MASK_ENTRIES] &=
+	    !(1 << (chip->dev_num % TPM_NUM_MASK_ENTRIES));
 
 	kfree(chip);
 
@@ -565,7 +562,6 @@ EXPORT_SYMBOL_GPL(tpm_pm_suspend);
 int tpm_pm_resume(struct pci_dev *pci_dev)
 {
 	struct tpm_chip *chip = pci_get_drvdata(pci_dev);
-
 	if (chip == NULL)
 		return -ENODEV;
 
@@ -585,8 +581,9 @@ EXPORT_SYMBOL_GPL(tpm_pm_resume);
  * upon errant exit from this function specific probe function should call
  * pci_disable_device
  */
-int tpm_register_hardware(struct pci_dev *pci_dev,
-			  struct tpm_vendor_specific *entry)
+int
+tpm_register_hardware(struct pci_dev *pci_dev,
+		      struct tpm_vendor_specific *entry)
 {
 	char devname[7];
 	struct tpm_chip *chip;
@@ -601,17 +598,21 @@ int tpm_register_hardware(struct pci_dev
 
 	init_MUTEX(&chip->buffer_mutex);
 	init_MUTEX(&chip->tpm_mutex);
-	init_MUTEX(&chip->timer_manipulation_mutex);
 	INIT_LIST_HEAD(&chip->list);
 
+	init_timer(&chip->user_read_timer);
+	chip->user_read_timer.function = user_reader_timeout;
+	chip->user_read_timer.data = (unsigned long) chip;
+
 	chip->vendor = entry;
 
 	chip->dev_num = -1;
 
-	for (i = 0; i < 32; i++)
-		for (j = 0; j < 8; j++)
+	for (i = 0; i < TPM_NUM_MASK_ENTRIES; i++)
+		for (j = 0; j < 8 * sizeof(int); j++)
 			if ((dev_mask[i] & (1 << j)) == 0) {
-				chip->dev_num = i * 32 + j;
+				chip->dev_num =
+				    i * TPM_NUM_MASK_ENTRIES + j;
 				dev_mask[i] |= 1 << j;
 				goto dev_num_search_complete;
 			}
@@ -633,12 +634,15 @@ int tpm_register_hardware(struct pci_dev
 	chip->vendor->miscdev.dev = &(pci_dev->dev);
 	chip->pci_dev = pci_dev_get(pci_dev);
 
+	spin_lock(&driver_lock);
+
 	if (misc_register(&chip->vendor->miscdev)) {
 		dev_err(&chip->pci_dev->dev,
 			"unable to misc_register %s, minor %d\n",
 			chip->vendor->miscdev.name,
 			chip->vendor->miscdev.minor);
 		pci_dev_put(pci_dev);
+		spin_unlock(&driver_lock);
 		kfree(chip);
 		dev_mask[i] &= !(1 << j);
 		return -ENODEV;
@@ -652,24 +656,12 @@ int tpm_register_hardware(struct pci_dev
 	device_create_file(&pci_dev->dev, &dev_attr_pcrs);
 	device_create_file(&pci_dev->dev, &dev_attr_caps);
 
+	spin_unlock(&driver_lock);
 	return 0;
 }
 
 EXPORT_SYMBOL_GPL(tpm_register_hardware);
 
-static int __init init_tpm(void)
-{
-	return 0;
-}
-
-static void __exit cleanup_tpm(void)
-{
-
-}
-
-module_init(init_tpm);
-module_exit(cleanup_tpm);
-
 MODULE_AUTHOR("Leendert van Doorn (leendert@watson.ibm.com)");
 MODULE_DESCRIPTION("TPM Driver");
 MODULE_VERSION("2.0");
diff -uprN linux-2.6.11.3-orig/drivers/char/tpm/tpm.h linux-2.6.11.3/drivers/char/tpm/tpm.h
--- linux-2.6.11.3-orig/drivers/char/tpm/tpm.h	2005-03-16 19:31:32.000000000 -0600
+++ linux-2.6.11.3/drivers/char/tpm/tpm.h	2005-03-16 19:31:32.000000000 -0600
@@ -24,11 +24,15 @@
 #include <linux/delay.h>
 #include <linux/miscdevice.h>
 
-#define TPM_TIMEOUT    5	/* msecs */
+enum {
+	TPM_TIMEOUT = 5		/* msecs */
+};
 
 /* TPM addresses */
-#define        TPM_ADDR                        0x4E
-#define        TPM_DATA                        0x4F
+enum {
+	TPM_ADDR = 0x4E,
+	TPM_DATA = 0x4F
+};
 
 struct tpm_chip;
 
@@ -57,8 +61,6 @@ struct tpm_chip {
 
 	struct timer_list user_read_timer;	/* user needs to claim result */
 	struct semaphore tpm_mutex;	/* tpm is processing */
-	struct timer_list device_timer;	/* tpm is processing */
-	struct semaphore timer_manipulation_mutex;
 
 	struct tpm_vendor_specific *vendor;
 
diff -uprN linux-2.6.11.3-orig/drivers/char/tpm/tpm_nsc.c linux-2.6.11.3/drivers/char/tpm/tpm_nsc.c
--- linux-2.6.11.3-orig/drivers/char/tpm/tpm_nsc.c	2005-03-16 19:31:32.000000000 -0600
+++ linux-2.6.11.3/drivers/char/tpm/tpm_nsc.c	2005-03-16 19:31:32.000000000 -0600
@@ -22,34 +22,42 @@
 #include "tpm.h"
 
 /* National definitions */
-#define        TPM_NSC_BASE                    0x360
-#define        TPM_NSC_IRQ                     0x07
+enum {
+	TPM_NSC_BASE = 0x360,
+	TPM_NSC_IRQ = 0x07
+};
 
-#define        NSC_LDN_INDEX                   0x07
-#define        NSC_SID_INDEX                   0x20
-#define        NSC_LDC_INDEX                   0x30
-#define        NSC_DIO_INDEX                   0x60
-#define        NSC_CIO_INDEX                   0x62
-#define        NSC_IRQ_INDEX                   0x70
-#define        NSC_ITS_INDEX                   0x71
-
-#define        NSC_STATUS                      0x01
-#define        NSC_COMMAND                     0x01
-#define        NSC_DATA                        0x00
+enum {
+	NSC_LDN_INDEX = 0x07,
+	NSC_SID_INDEX = 0x20,
+	NSC_LDC_INDEX = 0x30,
+	NSC_DIO_INDEX = 0x60,
+	NSC_CIO_INDEX = 0x62,
+	NSC_IRQ_INDEX = 0x70,
+	NSC_ITS_INDEX = 0x71
+};
 
-/* status bits */
-#define        NSC_STATUS_OBF                  0x01	/* output buffer full */
-#define        NSC_STATUS_IBF                  0x02	/* input buffer full */
-#define        NSC_STATUS_F0                   0x04	/* F0 */
-#define        NSC_STATUS_A2                   0x08	/* A2 */
-#define        NSC_STATUS_RDY                  0x10	/* ready to receive command */
-#define        NSC_STATUS_IBR                  0x20	/* ready to receive data */
+enum {
+	NSC_STATUS = 0x01,
+	NSC_COMMAND = 0x01,
+	NSC_DATA = 0x00
+};
 
+/* status bits */
+enum {
+	NSC_STATUS_OBF = 0x01,	/* output buffer full */
+	NSC_STATUS_IBF = 0x02,	/* input buffer full */
+	NSC_STATUS_F0 = 0x04,	/* F0 */
+	NSC_STATUS_A2 = 0x08,	/* A2 */
+	NSC_STATUS_RDY = 0x10,	/* ready to receive command */
+	NSC_STATUS_IBR = 0x20	/* ready to receive data */
+};
 /* command bits */
-#define        NSC_COMMAND_NORMAL              0x01	/* normal mode */
-#define        NSC_COMMAND_EOC                 0x03
-#define        NSC_COMMAND_CANCEL              0x22
-
+enum {
+	NSC_COMMAND_NORMAL = 0x01,	/* normal mode */
+	NSC_COMMAND_EOC = 0x03,
+	NSC_COMMAND_CANCEL = 0x22
+};
 /*
  * Wait for a certain status to appear
  */
diff -uprN linux-2.6.11.3-orig/MAINTAINERS linux-2.6.11.3/MAINTAINERS
--- linux-2.6.11.3-orig/MAINTAINERS	2005-03-13 00:44:28.000000000 -0600
+++ linux-2.6.11.3/MAINTAINERS	2005-03-16 19:08:54.000000000 -0600
@@ -2087,6 +2087,13 @@ M:	perex@suse.cz
 L:	alsa-devel@alsa-project.org
 S:	Maintained
 
+TPM DEVICE DRIVER
+P:	Kylene Hall
+M:	kjhall@us.ibm.com
+W:	http://tpmdd.sourceforge.net
+L:	tpmdd-devel@lists.sourceforge.net
+S:	Maintained
+
 UltraSPARC (sparc64):
 P:	David S. Miller
 M:	davem@davemloft.net

Re: [PATCH] Add TPM hardware enablement driver

[Jeff Garzik]

Kylene Hall wrote:
>>what is the purpose of this pci_dev_get/put?  attempting to prevent hotplug or
>>something?
> 
> 
> Seems that since there is a refernce to the device in the chip structure 
> and I am making the file private data pointer point to that chip structure 
> this is another reference that must be accounted for. If you remove it 
> with it open and attempt read or write bad things will happen.  This isn't 
> really hotpluggable either as the TPM is on the motherboard.

My point was that there will always be a reference -anyway-, AFAICS. 
There is a pci_dev reference assigned to the pci_driver when the PCI 
driver is loaded, and all uses by the TPM generic code of this pointer 
are -inside- the pci_driver's pci_dev object lifetime.


>>>+
>>>+	/* cannot perform a write until the read has cleared
>>>+	   either via tpm_read or a user_read_timer timeout */
>>>+	while (atomic_read(&chip->data_pending) != 0) {
>>>+		set_current_state(TASK_UNINTERRUPTIBLE);
>>>+		schedule_timeout(TPM_TIMEOUT);
>>
> 
>>use msleep()
> 
> 
> addressed in another patch by Nish
> 
> 
>>>+	/* atomic tpm command send and result receive */
>>>+	out_size = tpm_transmit(chip, chip->data_buffer, TPM_BUFSIZE);
>>
>>major bug?  in_size may be smaller than TPM_BUFSIZE
> 
> 
> chip->data_buffer is allocated in open and is always this size.  The 
> operation needs to be atomic so the big buffer is to cover the size of a 
> potentially larger result.  Only reading in_size from the user with 
> copy_from_user

You output -more- data than you have input.

AFAICS that's a security bug (data leak), unless you memset the data 
area beforehand.

	Jeff

Re: [PATCH] Add TPM hardware enablement driver

[Greg KH]

On Tue, Mar 22, 2005 at 09:02:24PM -0500, Jeff Garzik wrote:
> Kylene Hall wrote:
> >>what is the purpose of this pci_dev_get/put?  attempting to prevent 
> >>hotplug or
> >>something?
> >
> >
> >Seems that since there is a refernce to the device in the chip structure 
> >and I am making the file private data pointer point to that chip structure 
> >this is another reference that must be accounted for. If you remove it 
> >with it open and attempt read or write bad things will happen.  This isn't 
> >really hotpluggable either as the TPM is on the motherboard.
> 
> My point was that there will always be a reference -anyway-, AFAICS. 
> There is a pci_dev reference assigned to the pci_driver when the PCI 
> driver is loaded, and all uses by the TPM generic code of this pointer 
> are -inside- the pci_driver's pci_dev object lifetime.

Think of the following situation:
	- driver is bound to device.
	- userspace opens char dev node.
	- device is removed from the system (using fakephp I can do this
	  to _any_ pci device, even if it is on the motherboard.)
	- userspace writes to char dev node
	- driver attempts to access pci device structure that is no
	  longer present in memory.

Because of this open needs to get a reference to the pci device to
prevent oopses, or the driver needs to be aware of "device is now gone"
in some other manner.

thanks,

greg k-h

Re: [PATCH] Add TPM hardware enablement driver

[Jeff Garzik]

Greg KH wrote:
> On Tue, Mar 22, 2005 at 09:02:24PM -0500, Jeff Garzik wrote:
> 
>>Kylene Hall wrote:
>>
>>>>what is the purpose of this pci_dev_get/put?  attempting to prevent 
>>>>hotplug or
>>>>something?
>>>
>>>
>>>Seems that since there is a refernce to the device in the chip structure 
>>>and I am making the file private data pointer point to that chip structure 
>>>this is another reference that must be accounted for. If you remove it 
>>>with it open and attempt read or write bad things will happen.  This isn't 
>>>really hotpluggable either as the TPM is on the motherboard.
>>
>>My point was that there will always be a reference -anyway-, AFAICS. 
>>There is a pci_dev reference assigned to the pci_driver when the PCI 
>>driver is loaded, and all uses by the TPM generic code of this pointer 
>>are -inside- the pci_driver's pci_dev object lifetime.
> 
> 
> Think of the following situation:
> 	- driver is bound to device.
> 	- userspace opens char dev node.
> 	- device is removed from the system (using fakephp I can do this
> 	  to _any_ pci device, even if it is on the motherboard.)
> 	- userspace writes to char dev node
> 	- driver attempts to access pci device structure that is no
> 	  longer present in memory.
> 
> Because of this open needs to get a reference to the pci device to
> prevent oopses, or the driver needs to be aware of "device is now gone"
> in some other manner.

Thanks for explaining; agreed.

However, there appear to still be massive bugs in this area:

Consider the behavior of the chrdev if a PCI device has been unplugged. 
  It's still actively messing with the non-existent hardware, and never 
checks for dead h/w AFAICS.

	Jeff

Re: [PATCH] Add TPM hardware enablement driver

[Greg KH]

On Thu, Mar 24, 2005 at 04:04:25PM -0500, Jeff Garzik wrote:
> Greg KH wrote:
> >On Tue, Mar 22, 2005 at 09:02:24PM -0500, Jeff Garzik wrote:
> >
> >>Kylene Hall wrote:
> >>
> >>>>what is the purpose of this pci_dev_get/put?  attempting to prevent 
> >>>>hotplug or
> >>>>something?
> >>>
> >>>
> >>>Seems that since there is a refernce to the device in the chip structure 
> >>>and I am making the file private data pointer point to that chip 
> >>>structure this is another reference that must be accounted for. If you 
> >>>remove it with it open and attempt read or write bad things will happen. 
> >>>This isn't really hotpluggable either as the TPM is on the motherboard.
> >>
> >>My point was that there will always be a reference -anyway-, AFAICS. 
> >>There is a pci_dev reference assigned to the pci_driver when the PCI 
> >>driver is loaded, and all uses by the TPM generic code of this pointer 
> >>are -inside- the pci_driver's pci_dev object lifetime.
> >
> >
> >Think of the following situation:
> >	- driver is bound to device.
> >	- userspace opens char dev node.
> >	- device is removed from the system (using fakephp I can do this
> >	  to _any_ pci device, even if it is on the motherboard.)
> >	- userspace writes to char dev node
> >	- driver attempts to access pci device structure that is no
> >	  longer present in memory.
> >
> >Because of this open needs to get a reference to the pci device to
> >prevent oopses, or the driver needs to be aware of "device is now gone"
> >in some other manner.
> 
> Thanks for explaining; agreed.
> 
> However, there appear to still be massive bugs in this area:
> 
> Consider the behavior of the chrdev if a PCI device has been
> unplugged.  It's still actively messing with the non-existent
> hardware, and never checks for dead h/w AFAICS.

I agree, the driver should be fixed to handle this properly.

thanks,

greg k-h

Re: [PATCH] Add TPM hardware enablement driver

[Kylene Jo Hall]

On Thu, 2005-03-24 at 13:33 -0800, Greg KH wrote:
> On Thu, Mar 24, 2005 at 04:04:25PM -0500, Jeff Garzik wrote:
> > Greg KH wrote:
> > >On Tue, Mar 22, 2005 at 09:02:24PM -0500, Jeff Garzik wrote:
> > >
> > >>Kylene Hall wrote:
> > >>
> > >>>>what is the purpose of this pci_dev_get/put?  attempting to prevent 
> > >>>>hotplug or
> > >>>>something?
> > >>>
> > >>>
> > >>>Seems that since there is a refernce to the device in the chip structure 
> > >>>and I am making the file private data pointer point to that chip 
> > >>>structure this is another reference that must be accounted for. If you 
> > >>>remove it with it open and attempt read or write bad things will happen. 
> > >>>This isn't really hotpluggable either as the TPM is on the motherboard.
> > >>
> > >>My point was that there will always be a reference -anyway-, AFAICS. 
> > >>There is a pci_dev reference assigned to the pci_driver when the PCI 
> > >>driver is loaded, and all uses by the TPM generic code of this pointer 
> > >>are -inside- the pci_driver's pci_dev object lifetime.
> > >
> > >
> > >Think of the following situation:
> > >	- driver is bound to device.
> > >	- userspace opens char dev node.
> > >	- device is removed from the system (using fakephp I can do this
> > >	  to _any_ pci device, even if it is on the motherboard.)
> > >	- userspace writes to char dev node
> > >	- driver attempts to access pci device structure that is no
> > >	  longer present in memory.
> > >
> > >Because of this open needs to get a reference to the pci device to
> > >prevent oopses, or the driver needs to be aware of "device is now gone"
> > >in some other manner.
> > 
> > Thanks for explaining; agreed.
> > 
> > However, there appear to still be massive bugs in this area:
> > 
> > Consider the behavior of the chrdev if a PCI device has been
> > unplugged.  It's still actively messing with the non-existent
> > hardware, and never checks for dead h/w AFAICS.
> 
> I agree, the driver should be fixed to handle this properly.
> 

I have now played with the fakephp driver and have a better
understanding of these interactions, but I still have questions.  With
the current structure there is a problem because everything is
"cleaned-up" with the tpm_remove function even if userspace has the
device open when the tpm's slot is removed and then there are problems
on subsequent reads/writes. The get/put didn't really stop this from
happening.  Is it right to fix this by cleaning mostly up and placing a
flag in the read/write path to check for this condition?

This problem actually becomes more complicated.  Since the TPM lives on
the LPC bus and does not have it's own id we were in the process of
converting the driver to not use a pci_driver structure at all like the
example in drivers/char/watchdog/i8xx_tco.c.  This is desirable so that
the driver does not claim the id and other drivers can still find their
devices that also live on the LPC bus and thus share the same ID.
Without a pci_driver structure there is no probe or remove functions and
thus the driver is not alerted of the loss of hardware.  Any
recommendations of how to handle this situation?

Thanks,
Kylie

Re: [PATCH] Add TPM hardware enablement driver

[Kylene Jo Hall]

On Tue, 2005-04-05 at 11:14 -0500, Kylene Jo Hall wrote:
> On Thu, 2005-03-24 at 13:33 -0800, Greg KH wrote:
> > On Thu, Mar 24, 2005 at 04:04:25PM -0500, Jeff Garzik wrote:
> > > Greg KH wrote:
> > > >On Tue, Mar 22, 2005 at 09:02:24PM -0500, Jeff Garzik wrote:
> > > >
> > > >>Kylene Hall wrote:
> > > >>
> > > >>>>what is the purpose of this pci_dev_get/put?  attempting to prevent 
> > > >>>>hotplug or
> > > >>>>something?
> > > >>>
> > > >>>
> > > >>>Seems that since there is a refernce to the device in the chip structure 
> > > >>>and I am making the file private data pointer point to that chip 
> > > >>>structure this is another reference that must be accounted for. If you 
> > > >>>remove it with it open and attempt read or write bad things will happen. 
> > > >>>This isn't really hotpluggable either as the TPM is on the motherboard.
> > > >>
> > > >>My point was that there will always be a reference -anyway-, AFAICS. 
> > > >>There is a pci_dev reference assigned to the pci_driver when the PCI 
> > > >>driver is loaded, and all uses by the TPM generic code of this pointer 
> > > >>are -inside- the pci_driver's pci_dev object lifetime.
> > > >
> > > >
> > > >Think of the following situation:
> > > >	- driver is bound to device.
> > > >	- userspace opens char dev node.
> > > >	- device is removed from the system (using fakephp I can do this
> > > >	  to _any_ pci device, even if it is on the motherboard.)
> > > >	- userspace writes to char dev node
> > > >	- driver attempts to access pci device structure that is no
> > > >	  longer present in memory.
> > > >
> > > >Because of this open needs to get a reference to the pci device to
> > > >prevent oopses, or the driver needs to be aware of "device is now gone"
> > > >in some other manner.
> > > 
> > > Thanks for explaining; agreed.
> > > 
> > > However, there appear to still be massive bugs in this area:
> > > 
> > > Consider the behavior of the chrdev if a PCI device has been
> > > unplugged.  It's still actively messing with the non-existent
> > > hardware, and never checks for dead h/w AFAICS.
> > 
> > I agree, the driver should be fixed to handle this properly.
> > 
> 

Basically, what I need to figure out is how to solve both issues
simultaneously.  I need to not register a pci_driver as I would be
taking over an ID that is not unique to my device as well as get the
hotplugging correct (which i don't know how to do with out a pci_remove
function).

Thanks,

> I have now played with the fakephp driver and have a better
> understanding of these interactions, but I still have questions.  With
> the current structure there is a problem because everything is
> "cleaned-up" with the tpm_remove function even if userspace has the
> device open when the tpm's slot is removed and then there are problems
> on subsequent reads/writes. The get/put didn't really stop this from
> happening.  Is it right to fix this by cleaning mostly up and placing a
> flag in the read/write path to check for this condition?
> 
> This problem actually becomes more complicated.  Since the TPM lives on
> the LPC bus and does not have it's own id we were in the process of
> converting the driver to not use a pci_driver structure at all like the
> example in drivers/char/watchdog/i8xx_tco.c.  This is desirable so that
> the driver does not claim the id and other drivers can still find their
> devices that also live on the LPC bus and thus share the same ID.
> Without a pci_driver structure there is no probe or remove functions and
> thus the driver is not alerted of the loss of hardware.  Any
> recommendations of how to handle this situation?
> 
> Thanks,
> Kylie
> 

Re: [PATCH] Add TPM hardware enablement driver

[Ian Campbell]

[-- Attachment #1: Type: text/plain, Size: 825 bytes --]

On Fri, 2005-04-08 at 15:07 -0500, Kylene Jo Hall wrote:

> Basically, what I need to figure out is how to solve both issues
> simultaneously.  I need to not register a pci_driver as I would be
> taking over an ID that is not unique to my device as well as get the
> hotplugging correct (which i don't know how to do with out a pci_remove
> function).

Perhaps it makes sense to create a new lpc subsystem with an lpc bus
type. Then the PCI driver would register a new lpc bus and LPC device
drivers would register themselves with that. Or if the LPC is
independent of any PCI device the LPC "bridge" driver would just be
another driver to be loaded.

It sounds like it might end up similar to the i2c subsystem for example.

Ian.
-- 
Ian Campbell

Every time I think I know where it's at, they move it.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[PATCH: 1 of 12] Fix concerns with TPM driver -- use enums

[Kylene Hall]

On Wed, 9 Mar 2005, Jeff Garzik wrote:

> Greg KH wrote:

<snip>

> > +#define	TPM_MINOR			224	/* officially assigned
> > */
> > +
> > +#define	TPM_BUFSIZE			2048
> > +
> > +/* PCI configuration addresses */
> > +#define	PCI_GEN_PMCON_1			0xA0
> > +#define	PCI_GEN1_DEC			0xE4
> > +#define	PCI_LPC_EN			0xE6
> > +#define	PCI_GEN2_DEC			0xEC
> 
> enums preferred to #defines, as these provide more type information, and are
> more visible in a debugger.
> 

<snip>

> 
> > +static int dev_mask[32];
> 
> don't use '32', create a constant and use that constant instead.
> 

<snip>

> 
> > +		tpm_write_index(0x0D, 0x55);	/* unlock 4F */
> > +		tpm_write_index(0x0A, 0x00);	/* int disable */
> > +		tpm_write_index(0x08, base);	/* base addr lo */
> > +		tpm_write_index(0x09, (base & 0xFF00) >> 8);	/* base addr
> > hi */
> > +		tpm_write_index(0x0D, 0xAA);	/* lock 4F */
> 
> please define symbol names for the TPM registers
> 

<snip>

> > +#define TPM_TIMEOUT msecs_to_jiffies(5)
> > +
> > +/* TPM addresses */
> > +#define	TPM_ADDR			0x4E
> > +#define	TPM_DATA			0x4F
> 
> enum preferred to #define

<snip>

> > +/* Atmel definitions */
> > +#define	TPM_ATML_BASE			0x400
> > +
> > +/* write status bits */
> > +#define	ATML_STATUS_ABORT		0x01
> > +#define	ATML_STATUS_LASTBYTE		0x04
> > +
> > +/* read status bits */
> > +#define	ATML_STATUS_BUSY		0x01
> > +#define	ATML_STATUS_DATA_AVAIL		0x02
> > +#define	ATML_STATUS_REWRITE		0x04
> 
> enum preferred

<snip>

> > +
> > +/* National definitions */
> > +#define	TPM_NSC_BASE			0x360
> > +#define	TPM_NSC_IRQ			0x07
> > +
> > +#define	NSC_LDN_INDEX			0x07
> > +#define	NSC_SID_INDEX			0x20
> > +#define	NSC_LDC_INDEX			0x30
> > +#define	NSC_DIO_INDEX			0x60
> > +#define	NSC_CIO_INDEX			0x62
> > +#define	NSC_IRQ_INDEX			0x70
> > +#define	NSC_ITS_INDEX			0x71
> > +
> > +#define	NSC_STATUS			0x01
> > +#define	NSC_COMMAND			0x01
> > +#define	NSC_DATA			0x00
> > +
> > +/* status bits */
> > +#define	NSC_STATUS_OBF			0x01	/* output buffer full
> > */
> > +#define	NSC_STATUS_IBF			0x02	/* input buffer full
> > */
> > +#define	NSC_STATUS_F0			0x04	/* F0 */
> > +#define	NSC_STATUS_A2			0x08	/* A2 */
> > +#define	NSC_STATUS_RDY			0x10	/* ready to receive
> > command */
> > +#define	NSC_STATUS_IBR			0x20	/* ready to receive
> > data */
> > +
> > +/* command bits */
> > +#define	NSC_COMMAND_NORMAL		0x01	/* normal mode */
> > +#define	NSC_COMMAND_EOC			0x03
> > +#define	NSC_COMMAND_CANCEL		0x22
> 
> enum

<snip>

The following patch addresses all of these issues where a symbol should 
have been used and an enum was preferable to a #define.  To apply cleanly 
the patch needs to be applied after the msleep fix patch submitted by Nish 
Aravamudan on March 10.

Signed-off-by: Kylene Hall <kjhall@us.ibm.com>
---
diff -uprN linux-2.6.12-rc2/drivers/char/tpm/tpm_atmel.c linux-2.6.12-rc2-tpmdd/drivers/char/tpm/tpm_atmel.c
--- linux-2.6.12-rc2/drivers/char/tpm/tpm_atmel.c	2005-04-15 16:31:21.000000000 -0500
+++ linux-2.6.12-rc2-tpmdd/drivers/char/tpm/tpm_atmel.c	2005-04-15 16:26:17.000000000 -0500
@@ -22,17 +22,22 @@
 #include "tpm.h"
 
 /* Atmel definitions */
-#define	TPM_ATML_BASE			0x400
+enum {
+	TPM_ATML_BASE = 0x400
+};
 
 /* write status bits */
-#define	ATML_STATUS_ABORT		0x01
-#define	ATML_STATUS_LASTBYTE		0x04
-
+enum {
+	ATML_STATUS_ABORT = 0x01,
+	ATML_STATUS_LASTBYTE = 0x04
+};
 /* read status bits */
-#define	ATML_STATUS_BUSY		0x01
-#define	ATML_STATUS_DATA_AVAIL		0x02
-#define	ATML_STATUS_REWRITE		0x04
-
+enum {
+	ATML_STATUS_BUSY = 0x01,
+	ATML_STATUS_DATA_AVAIL = 0x02,
+	ATML_STATUS_REWRITE = 0x04,
+	ATML_STATUS_READY = 0x08
+};

 static int tpm_atml_recv(struct tpm_chip *chip, u8 * buf, size_t count)
 {
diff -uprN linux-2.6.12-rc2/drivers/char/tpm/tpm.c linux-2.6.12-rc2-tpmdd/drivers/char/tpm/tpm.c
--- linux-2.6.12-rc2/drivers/char/tpm/tpm.c	2005-04-15 16:30:55.000000000 -0500
+++ linux-2.6.12-rc2-tpmdd/drivers/char/tpm/tpm.c	2005-04-15 16:28:55.000000000 -0500
@@ -28,19 +28,35 @@
 #include <linux/spinlock.h>
 #include "tpm.h"
 
-#define	TPM_MINOR			224	/* officially assigned */
+enum {
+	TPM_MINOR = 224,	/* officially assigned */
+	TPM_BUFSIZE = 2048,
+	TPM_NUM_DEVICES = 256,
+	TPM_NUM_MASK_ENTRIES = TPM_NUM_DEVICES / (8 * sizeof(int))
+};
 
-#define	TPM_BUFSIZE			2048
+  /* PCI configuration addresses */
+enum {
+	PCI_GEN_PMCON_1 = 0xA0,
+	PCI_GEN1_DEC = 0xE4,
+	PCI_LPC_EN = 0xE6,
+	PCI_GEN2_DEC = 0xEC
+};
+
+enum {
+	TPM_LOCK_REG = 0x0D,
+	TPM_INTERUPT_REG = 0x0A,
+	TPM_BASE_ADDR_LO = 0x08,
+	TPM_BASE_ADDR_HI = 0x09,
+	TPM_UNLOCK_VALUE = 0x55,
+	TPM_LOCK_VALUE = 0xAA,
+	TPM_DISABLE_INTERUPT_VALUE = 0x00
+};
 
-/* PCI configuration addresses */
-#define	PCI_GEN_PMCON_1			0xA0
-#define	PCI_GEN1_DEC			0xE4
-#define	PCI_LPC_EN			0xE6
-#define	PCI_GEN2_DEC			0xEC
 
 static LIST_HEAD(tpm_chip_list);
 static DEFINE_SPINLOCK(driver_lock);
-static int dev_mask[32];
+static int dev_mask[TPM_NUM_MASK_ENTRIES];
 
 static void user_reader_timeout(unsigned long ptr)
 {
@@ -102,11 +118,11 @@ int tpm_lpc_bus_init(struct pci_dev *pci
 			pci_write_config_dword(pci_dev, PCI_GEN_PMCON_1,
 					       tmp);
 		}
-		tpm_write_index(0x0D, 0x55);	/* unlock 4F */
-		tpm_write_index(0x0A, 0x00);	/* int disable */
-		tpm_write_index(0x08, base);	/* base addr lo */
-		tpm_write_index(0x09, (base & 0xFF00) >> 8);	/* base addr hi */
-		tpm_write_index(0x0D, 0xAA);	/* lock 4F */
+		tpm_write_index(TPM_LOCK_REG, TPM_UNLOCK_VALUE);
+		tpm_write_index(TPM_INTERUPT_REG, TPM_DISABLE_INTERUPT_VALUE);
+		tpm_write_index(TPM_BASE_ADDR_LO, base);
+		tpm_write_index(TPM_BASE_ADDR_HI, (base & 0xFF00) >> 8); 
+		tpm_write_index(TPM_LOCK_REG, TPM_LOCK_VALUE);
 		break;
 	case PCI_VENDOR_ID_AMD:
 		/* nothing yet */
diff -uprN linux-2.6.12-rc2/drivers/char/tpm/tpm_nsc.c linux-2.6.12-rc2-tpmdd/drivers/char/tpm/tpm_nsc.c
--- linux-2.6.12-rc2/drivers/char/tpm/tpm_nsc.c	2005-04-15 16:31:31.000000000 -0500
+++ linux-2.6.12-rc2-tpmdd/drivers/char/tpm/tpm_nsc.c	2005-04-15 16:26:28.000000000 -0500
@@ -22,34 +22,42 @@
 #include "tpm.h"
 
 /* National definitions */
-#define	TPM_NSC_BASE			0x360
-#define	TPM_NSC_IRQ			0x07
+enum {
+	TPM_NSC_BASE = 0x360,
+	TPM_NSC_IRQ = 0x07
+};
 
-#define	NSC_LDN_INDEX			0x07
-#define	NSC_SID_INDEX			0x20
-#define	NSC_LDC_INDEX			0x30
-#define	NSC_DIO_INDEX			0x60
-#define	NSC_CIO_INDEX			0x62
-#define	NSC_IRQ_INDEX			0x70
-#define	NSC_ITS_INDEX			0x71
-
-#define	NSC_STATUS			0x01
-#define	NSC_COMMAND			0x01
-#define	NSC_DATA			0x00
+enum {
+	NSC_LDN_INDEX = 0x07,
+	NSC_SID_INDEX = 0x20,
+	NSC_LDC_INDEX = 0x30,
+	NSC_DIO_INDEX = 0x60,
+	NSC_CIO_INDEX = 0x62,
+	NSC_IRQ_INDEX = 0x70,
+	NSC_ITS_INDEX = 0x71
+};
 
-/* status bits */
-#define	NSC_STATUS_OBF			0x01	/* output buffer full */
-#define	NSC_STATUS_IBF			0x02	/* input buffer full */
-#define	NSC_STATUS_F0			0x04	/* F0 */
-#define	NSC_STATUS_A2			0x08	/* A2 */
-#define	NSC_STATUS_RDY			0x10	/* ready to receive command */
-#define	NSC_STATUS_IBR			0x20	/* ready to receive data */
+enum {
+	NSC_STATUS = 0x01,
+	NSC_COMMAND = 0x01,
+	NSC_DATA = 0x00
+};
 
+/* status bits */
+enum {
+	NSC_STATUS_OBF = 0x01,	/* output buffer full */
+	NSC_STATUS_IBF = 0x02,	/* input buffer full */
+	NSC_STATUS_F0 = 0x04,	/* F0 */
+	NSC_STATUS_A2 = 0x08,	/* A2 */
+	NSC_STATUS_RDY = 0x10,	/* ready to receive command */
+	NSC_STATUS_IBR = 0x20	/* ready to receive data */
+};
 /* command bits */
-#define	NSC_COMMAND_NORMAL		0x01	/* normal mode */
-#define	NSC_COMMAND_EOC			0x03
-#define	NSC_COMMAND_CANCEL		0x22
-
+enum {
+	NSC_COMMAND_NORMAL = 0x01,	/* normal mode */
+	NSC_COMMAND_EOC = 0x03,
+	NSC_COMMAND_CANCEL = 0x22
+};
 /*
  * Wait for a certain status to appear
  */
+};
diff -uprN linux-2.6.12-rc2/drivers/char/tpm/tpm.h linux-2.6.12-rc2-tpmdd/drivers/char/tpm/tpm.h
--- linux-2.6.12-rc2/drivers/char/tpm/tpm.h     2005-04-15 15:13:29.000000000 -0500
+++ linux-2.6.12-rc2-tpmdd/drivers/char/tpm/tpm.h       2005-04-15 16:25:18.000000000 -0500
@@ -25,11 +25,17 @@
 #include <linux/fs.h>
 #include <linux/miscdevice.h>
 
-#define TPM_TIMEOUT    5       /* msecs */
+enum {
+	TPM_TIMEOUT = 5,	/* msecs */
+	TPM_NUM_ATTR = 4
+};
 
 /* TPM addresses */
-#define	TPM_ADDR			0x4E
-#define	TPM_DATA			0x4F
+enum {
+	TPM_ADDR = 0x4E,
+	TPM_DATA = 0x4F
+};
+
 
 struct tpm_chip;
 
--- linux-2.6.12-rc2/drivers/char/tpm/tpm.c	2005-04-25 18:44:16.000000000 -0500
+++ linux-2.6.12-rc2-tpmdd/drivers/chat/tpm/tpm.c	2005-04-25 18:45:30.000000000 -0500
@@ -566,7 +566,7 @@ void __devexit tpm_remove(struct pci_dev
 
 	pci_disable_device(pci_dev);
 
-	dev_mask[chip->dev_num / 32] &= !(1 << (chip->dev_num % 32));
+	dev_mask[chip->dev_num / TPM_NUM_MASK_ENTRIES ] &= !(1 << (chip->dev_num % TPM_NUM_MASK_ENTRIES));
 
 	kfree(chip);
 
@@ -646,10 +646,11 @@ int tpm_register_hardware(struct pci_dev
 
 	chip->dev_num = -1;
 
-	for (i = 0; i < 32; i++)
-		for (j = 0; j < 8; j++)
+	for (i = 0; i < TPM_NUM_MASK_ENTRIES; i++)
+		for (j = 0; j < 8 * sizeof(int); j++)
 			if ((dev_mask[i] & (1 << j)) == 0) {
-				chip->dev_num = i * 32 + j;
+				chip->dev_num =
+				    i * TPM_NUM_MASK_ENTRIES + j;
 				dev_mask[i] |= 1 << j;
 				goto dev_num_search_complete;
 			}

Re: [PATCH: 1 of 12] Fix concerns with TPM driver -- use enums

[Greg KH]

On Wed, Apr 27, 2005 at 05:15:46PM -0500, Kylene Hall wrote:
>  /* Atmel definitions */
> -#define	TPM_ATML_BASE			0x400
> +enum {
> +	TPM_ATML_BASE = 0x400
> +};

Please name your enumerated types, so that you can try to check for
type-safeness on them.  Just converting them to a enum doesn't do really
anything...

thanks,

greg k-h

[PATCH: 2 of 12 ] Fix TPM driver -- address missing const defs

[Kylene Hall]

On Wed, 9 Mar 2005, Jeff Garzik wrote:
> Greg KH wrote:
<snip>

> > +static u8 cap_pcr[] = {
> > +	0, 193,			/* TPM_TAG_RQU_COMMAND */
> > +	0, 0, 0, 22,		/* length */
> > +	0, 0, 0, 101,		/* TPM_ORD_GetCapability */
> > +	0, 0, 0, 5,
> > +	0, 0, 0, 4,
> > +	0, 0, 1, 1
> > +};
> 
> const
> 
> 
> > +#define READ_PCR_RESULT_SIZE 30
> > +static u8 pcrread[] = {
> > +	0, 193,			/* TPM_TAG_RQU_COMMAND */
> > +	0, 0, 0, 14,		/* length */
> > +	0, 0, 0, 21,		/* TPM_ORD_PcrRead */
> > +	0, 0, 0, 0		/* PCR index */
> > +};
> 
> const

<snip>

> > +static u8 cap_version[] = {
> > +	0, 193,			/* TPM_TAG_RQU_COMMAND */
> > +	0, 0, 0, 18,		/* length */
> > +	0, 0, 0, 101,		/* TPM_ORD_GetCapability */
> > +	0, 0, 0, 6,
> > +	0, 0, 0, 0
> > +};
> 
> const
> 
> 
> > +#define CAP_MANUFACTURER_RESULT_SIZE 18

> > +static u8 cap_manufacturer[] = {
> > +	0, 193,			/* TPM_TAG_RQU_COMMAND */
> > +	0, 0, 0, 22,		/* length */
> > +	0, 0, 0, 101,		/* TPM_ORD_GetCapability */
> > +	0, 0, 0, 5,
> > +	0, 0, 0, 4,
> > +	0, 0, 1, 3
> > +};
> 
> const
> 

<snip>

> > +static u8 savestate[] = {
> > +	0, 193,			/* TPM_TAG_RQU_COMMAND */
> > +	0, 0, 0, 10,		/* blob length (in bytes) */
> > +	0, 0, 0, 152		/* TPM_ORD_SaveState */
> > +};
> 
> const

<snip>

The following patch addresses the need for all of this missing const 
definitions.

Signed-off-by: Kylene Hall <kjhall@us.ibm.com>
---
diff -uprN --exclude='*.orig' linux-2.6.12-rc2/drivers/char/tpm/tpm.c linux-2.6.12-rc2-tpmdd/drivers/char/tpm/tpm.c
--- linux-2.6.12-rc2/drivers/char/tpm/tpm.c	2005-04-18 18:40:07.000000000 -0500
+++ linux-2.6.12-rc2-tpmdd/drivers/char/tpm/tpm.c	2005-04-20 17:11:43.000000000 -0500
@@ -192,7 +192,7 @@ out_recv:
 
 #define TPM_DIGEST_SIZE 20
 #define CAP_PCR_RESULT_SIZE 18
-static u8 cap_pcr[] = {
+static const u8 cap_pcr[] = {
 	0, 193,			/* TPM_TAG_RQU_COMMAND */
 	0, 0, 0, 22,		/* length */
 	0, 0, 0, 101,		/* TPM_ORD_GetCapability */
@@ -202,7 +202,7 @@ static u8 cap_pcr[] = {
 };
 
 #define READ_PCR_RESULT_SIZE 30
-static u8 pcrread[] = {
+static const u8 pcrread[] = {
 	0, 193,			/* TPM_TAG_RQU_COMMAND */
 	0, 0, 0, 14,		/* length */
 	0, 0, 0, 21,		/* TPM_ORD_PcrRead */
@@ -246,7 +246,7 @@ static ssize_t show_pcrs(struct device *
 static DEVICE_ATTR(pcrs, S_IRUGO, show_pcrs, NULL);
 
 #define  READ_PUBEK_RESULT_SIZE 314
-static u8 readpubek[] = {
+static const u8 readpubek[] = {
 	0, 193,			/* TPM_TAG_RQU_COMMAND */
 	0, 0, 0, 30,		/* length */
 	0, 0, 0, 124,		/* TPM_ORD_ReadPubek */
@@ -309,7 +309,7 @@ static ssize_t show_pubek(struct device 
 static DEVICE_ATTR(pubek, S_IRUGO, show_pubek, NULL);
 
 #define CAP_VER_RESULT_SIZE 18
-static u8 cap_version[] = {
+static const u8 cap_version[] = {
 	0, 193,			/* TPM_TAG_RQU_COMMAND */
 	0, 0, 0, 18,		/* length */
 	0, 0, 0, 101,		/* TPM_ORD_GetCapability */
@@ -318,7 +318,7 @@ static u8 cap_version[] = {
 };
 
 #define CAP_MANUFACTURER_RESULT_SIZE 18
-static u8 cap_manufacturer[] = {
+static const u8 cap_manufacturer[] = {
 	0, 193,			/* TPM_TAG_RQU_COMMAND */
 	0, 0, 0, 22,		/* length */
 	0, 0, 0, 101,		/* TPM_ORD_GetCapability */

[PATCH: 3 of 12] Fix TPM driver --remove unnecessary module stuff

[Kylene Hall]

On Wed, 9 Mar 2005, Jeff Garzik wrote:
> Greg KH wrote:

<snip>

> > +
> > +static int __init init_tpm(void)
> > +{
> > +	return 0;
> > +}
> > +
> > +static void __exit cleanup_tpm(void)
> > +{
> > +
> > +}
> > +
> > +module_init(init_tpm);
> > +module_exit(cleanup_tpm);
> 
> why?  just delete these, I would say.
> 

<snip>

No reason for these module definitions.  This patch removes them.

Signed-off-by: Kylene Hall <kjhall@us.ibm.com>
---
--- linux-2.6.12-rc2/drivers/chat/tpm/tpm.c	2005-04-21 17:45:30.000000000 -0500
+++ linux-2.6.12-rc2-tpmdd/drivers/char/tpm/tpm.c	2005-04-21 17:36:59.000000000 -0500
@@ -668,19 +668,6 @@ dev_num_search_complete:
 
 EXPORT_SYMBOL_GPL(tpm_register_hardware);
 
-static int __init init_tpm(void)
-{
-	return 0;
-}
-
-static void __exit cleanup_tpm(void)
-{
-
-}
-
-module_init(init_tpm);
-module_exit(cleanup_tpm);
-
 MODULE_AUTHOR("Leendert van Doorn (leendert@watson.ibm.com)");
 MODULE_DESCRIPTION("TPM Driver");
 MODULE_VERSION("2.0");

[PATCH 4 of 12] Fix TPM driver -- read return code issue

[Kylene Hall]

On Wed, 9 Mar 2005, Jeff Garzik wrote:
> Greg KH wrote:

<snip>

> > +ssize_t tpm_read(struct file * file, char __user * buf,
> > +		 size_t size, loff_t * off)
> > +{
> > +	struct tpm_chip *chip = file->private_data;
> > +	int ret_size = -ENODATA;
> > +
> > +	if (atomic_read(&chip->data_pending) != 0) {	/* Result available */
> > +		down(&chip->timer_manipulation_mutex);
> > +		del_singleshot_timer_sync(&chip->user_read_timer);
> > +		up(&chip->timer_manipulation_mutex);
> > +
> > +		down(&chip->buffer_mutex);
> > +
> > +		ret_size = atomic_read(&chip->data_pending);
> > +		atomic_set(&chip->data_pending, 0);
> > +
> > +		if (ret_size == 0)	/* timeout just occurred */
> > +			ret_size = -ETIME;
> > +		else if (ret_size > 0) {	/* relay data */
> > +			if (size < ret_size)
> > +				ret_size = size;
> > +
> > +			if (copy_to_user((void __user *) buf,
> > +					 chip->data_buffer, ret_size)) {
> > +				ret_size = -EFAULT;
> > +			}
> > +		}
> > +		up(&chip->buffer_mutex);
> > +	}
> > +
> > +	return ret_size;
> 
> POSIX violation -- when there is no data available, returning a non-standard
> error is silly

<snip>

The patch below fixes this erroneous return code when no data is 
available.

Signed-of-by: Kylene Hall <kjhall@us.ibm.com>
---
--- linux-2.6.12-rc2/drivers/char/tpm/tpm.c	2005-04-21 17:36:59.000000000 -0500
+++ linux-2.6.12-rc2-tpmdd/drivers/char/tpm/tpm.c	2005-04-21 17:57:39.000000000 -0500
@@ -483,29 +483,19 @@ ssize_t tpm_read(struct file * file, cha
 		 size_t size, loff_t * off)
 {
 	struct tpm_chip *chip = file->private_data;
-	int ret_size = -ENODATA;
+	int ret_size;
 
-	if (atomic_read(&chip->data_pending) != 0) {	/* Result available */
-		down(&chip->timer_manipulation_mutex);
-		del_singleshot_timer_sync(&chip->user_read_timer);
-		up(&chip->timer_manipulation_mutex);
+	del_singleshot_timer_sync(&chip->user_read_timer);
+	ret_size = atomic_read(&chip->data_pending);
+	atomic_set(&chip->data_pending, 0);
+	if (ret_size > 0) {	/* relay data */
+		if (size < ret_size)
+			ret_size = size;
 
 		down(&chip->buffer_mutex);
-
-		ret_size = atomic_read(&chip->data_pending);
-		atomic_set(&chip->data_pending, 0);
-
-		if (ret_size == 0)	/* timeout just occurred */
-			ret_size = -ETIME;
-		else if (ret_size > 0) {	/* relay data */
-			if (size < ret_size)
-				ret_size = size;
-
-			if (copy_to_user((void __user *) buf,
-					 chip->data_buffer, ret_size)) {
-				ret_size = -EFAULT;
-			}
-		}
+		if (copy_to_user
+		    ((void __user *) buf, chip->data_buffer, ret_size))
+			ret_size = -EFAULT;
 		up(&chip->buffer_mutex);
 	}
 

[PATCH 5 of 12] Fix TPM driver -- large stack objects

[Kylene Hall]

On Wed, 9 Mar 2005, Jeff Garzik wrote:
> Greg KH wrote:

<snip>

> > +static ssize_t show_pubek(struct device *dev, char *buf)
> > +{
> > +	u8 data[READ_PUBEK_RESULT_SIZE];
> 
> massive obj on stack

<snip>

> 
> > +static ssize_t show_caps(struct device *dev, char *buf)
> > +{
> > +	u8 data[READ_PUBEK_RESULT_SIZE];
> 
> massive obj on stack

<snip>

The patch below containes fixes for these large stack objects.

Signed-off-by: Kylene Hall <kjhall@us.ibm.com>
---
--- linux-2.6.12-rc2/drivers/char/tpm/tpm.c	2005-04-21 18:11:12.000000000 -0500
+++ linux-2.6.12-rc2-tpmdd/drivers/char/tpm/tpm.c	2005-04-21 18:13:10.000000000 -0500
@@ -253,7 +253,7 @@ static const u8 readpubek[] = {
 
 ssize_t tpm_show_pubek(struct device *dev, char *buf)
 {
-	u8 data[READ_PUBEK_RESULT_SIZE];
+	u8 *data;
 	ssize_t len;
 	int i, rc;
 	char *str = buf;
@@ -263,12 +263,18 @@ ssize_t tpm_show_pubek(struct device *de
 	if (chip == NULL)
 		return -ENODEV;
 
+	data = kmalloc(READ_PUBEK_RESULT_SIZE, GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
 	memcpy(data, readpubek, sizeof(readpubek));
 	memset(data + sizeof(readpubek), 0, 20);	/* zero nonce */
 
-	if ((len = tpm_transmit(chip, data, sizeof(data))) <
-	    READ_PUBEK_RESULT_SIZE)
-		return len;
+	if ((len = tpm_transmit(chip, data, READ_PUBEK_RESULT_SIZE)) <
+	    READ_PUBEK_RESULT_SIZE) {
+		rc = len;
+		goto out;
+	}
 
 	/* 
 	   ignore header 10 bytes
@@ -298,7 +304,10 @@ ssize_t tpm_show_pubek(struct device *de
 		if ((i + 1) % 16 == 0)
 			str += sprintf(str, "\n");
 	}
-	return str - buf;
+	rc = str - buf;
+out:
+	kfree(data);
+	return rc;
 }
 
 EXPORT_SYMBOL_GPL(tpm_show_pubek);
@@ -324,7 +333,7 @@ static const u8 cap_manufacturer[] = {
 
 ssize_t tpm_show_caps(struct device *dev, char *buf)
 {
-	u8 data[READ_PUBEK_RESULT_SIZE];
+	u8 data[sizeof(cap_manufacturer)];
 	ssize_t len;
 	char *str = buf;
 

[PATCH 6 of 12] Fix TPM driver -- how timer is initialized

[Kylene Hall]

On Wed, 9 Mar 2005, Jeff Garzik wrote:
> Greg KH wrote:

<snip>
> > +
> > +	down(&chip->timer_manipulation_mutex);
> > +	chip->time_expired = 0;
> > +	init_timer(&chip->device_timer);
> > +	chip->device_timer.function = tpm_time_expired;
> > +	chip->device_timer.expires = jiffies + 2 * 60 * HZ;
> > +	chip->device_timer.data = (unsigned long) &chip->time_expired;
> > +	add_timer(&chip->device_timer);
> 
> very wrong.  you init_timer() when you initialize 'chip'... once.  then during
> the device lifetime you add/mod/del the timer.
> 
> calling init_timer() could lead to corruption of state.
> 

<snip>

> > +	/* Set a timeout by which the reader must come claim the result */
> > +	down(&chip->timer_manipulation_mutex);
> > +	init_timer(&chip->user_read_timer);
> > +	chip->user_read_timer.function = user_reader_timeout;
> > +	chip->user_read_timer.data = (unsigned long) chip;
> > +	chip->user_read_timer.expires = jiffies + (60 * HZ);
> > +	add_timer(&chip->user_read_timer);
> 
> again, don't repeatedly init_timer()

<snip>

> > +int tpm_register_hardware(struct pci_dev *pci_dev,
> > +			  struct tpm_vendor_specific *entry)
> > +{
> > +	char devname[7];
> > +	struct tpm_chip *chip;
> > +	int i, j;
> > +
> > +	/* Driver specific per-device data */
> > +	chip = kmalloc(sizeof(*chip), GFP_KERNEL);
> > +	if (chip == NULL)
> > +		return -ENOMEM;
> > +
> > +	memset(chip, 0, sizeof(struct tpm_chip));
> > +

> > +	init_MUTEX(&chip->buffer_mutex);
> > +	init_MUTEX(&chip->tpm_mutex);
> > +	init_MUTEX(&chip->timer_manipulation_mutex);
> > +	INIT_LIST_HEAD(&chip->list);
> 
> timer init should be here

<snip>

Fix the timer to be inited and modified properly.  This work depends on 
the fixing of the msleep stuff which was submitted in a patch by Nish 
Aravamudan on March 10.

Signed-of-by: Kylene Hall <kjhall@us.ibm.com>
---
diff -urpN --exclude='*.orig' linux-2.6.12-rc2/drivers/char/tpm/tpm.c linux-2.6.12-rc2-tpmdd/drivers/char/tpm/tpm.c
--- linux-2.6.12-rc2/drivers/char/tpm/tpm.c	2005-04-25 18:49:08.000000000 -0500
+++ linux-2.6.12-rc2-tpmdd/drivers/char/tpm/tpm.c	2005-04-26 12:47:20.000000000 -0500
@@ -456,16 +456,7 @@ int tpm_release(struct inode *inode, str
 
 	spin_lock(&driver_lock);
 	chip->num_opens--;
-	spin_unlock(&driver_lock);
-
-	down(&chip->timer_manipulation_mutex);
-	if (timer_pending(&chip->user_read_timer))
-		del_singleshot_timer_sync(&chip->user_read_timer);
-	else if (timer_pending(&chip->device_timer))
-		del_singleshot_timer_sync(&chip->device_timer);
-	up(&chip->timer_manipulation_mutex);
-
-	kfree(chip->data_buffer);
+	del_singleshot_timer_sync(&chip->user_read_timer);
 	atomic_set(&chip->data_pending, 0);
 
 	pci_dev_put(chip->pci_dev);
@@ -504,13 +495,7 @@ tpm_write(struct file * file, const char
 	up(&chip->buffer_mutex);
 
 	/* Set a timeout by which the reader must come claim the result */
-	down(&chip->timer_manipulation_mutex);
-	init_timer(&chip->user_read_timer);
-	chip->user_read_timer.function = user_reader_timeout;
-	chip->user_read_timer.data = (unsigned long) chip;
-	chip->user_read_timer.expires = jiffies + (60 * HZ);
-	add_timer(&chip->user_read_timer);
-	up(&chip->timer_manipulation_mutex);
+	mod_timer(&chip->user_read_timer, jiffies + (60 * HZ));
 
 	return in_size;
 }
@@ -639,9 +624,12 @@ int tpm_register_hardware(struct pci_dev
 
 	init_MUTEX(&chip->buffer_mutex);
 	init_MUTEX(&chip->tpm_mutex);
-	init_MUTEX(&chip->timer_manipulation_mutex);
 	INIT_LIST_HEAD(&chip->list);
 
+	init_timer(&chip->user_read_timer);
+	chip->user_read_timer.function = user_reader_timeout;
+	chip->user_read_timer.data = (unsigned long) chip;
+
 	chip->vendor = entry;
 
 	chip->dev_num = -1;
diff -urpN --exclude='*.orig' linux-2.6.12-rc2/drivers/char/tpm/tpm.h linux-2.6.12-rc2-tpmdd/drivers/char/tpm/tpm.h
--- linux-2.6.12-rc2/drivers/char/tpm/tpm.h	2005-04-25 18:49:08.000000000 -0500
+++ linux-2.6.12-rc2-tpmdd/drivers/char/tpm/tpm.h	2005-04-26 12:53:29.000000000 -0500
@@ -76,8 +76,6 @@ struct tpm_chip {
 
 	struct timer_list user_read_timer;	/* user needs to claim result */
 	struct semaphore tpm_mutex;	/* tpm is processing */
-	struct timer_list device_timer;	/* tpm is processing */
-	struct semaphore timer_manipulation_mutex;
 
 	struct tpm_vendor_specific *vendor;
 

[PATCH 7 of 12] Fix TPM driver -- use to_pci_dev

[Kylene Hall]

On Wed, 9 Mar 2005, Jeff Garzik wrote:
> Greg KH wrote:

<snip>

> > +static ssize_t show_pcrs(struct device *dev, char *buf)
> > +{
> > +	u8 data[READ_PCR_RESULT_SIZE];
> > +	ssize_t len;
> > +	int i, j, index, num_pcrs;
> > +	char *str = buf;
> > +
> > +	struct tpm_chp *chip =
> > +	    pci_get_drvdata(container_of(dev, struct pci_dev, dev));
> 
> use to_pci_dev()

<snip>

> > +	ssize_t len;
> > +	__be32 *native_val;
> > +	int i;
> > +	char *str = buf;
> > +
> > +	struct tpm_chip *chip =
> > +	    pci_get_drvdata(container_of(dev, struct pci_dev, dev));
> 
> to_pci_dev()

<snip>

> > +	ssize_t len;
> > +	char *str = buf;
> > +
> > +	struct tpm_chip *chip =
> > +	    pci_get_drvdata(container_of(dev, struct pci_dev, dev));
> 
> to_pci_dev()

<snip>

The following patch changes these container_of calls to 'to_pci_dev' as 
suggested above.

Signed-off-by: Kylene Hall <kjhall@us.ibm.com>
---
--- linux-2.6.12-rc2/drivers/char/tpm/tpm.c	2005-04-26 16:45:51.000000000 -0500
+++ linux-2.6.12-rc2-tpmdd/drivers/char/tpm/tpm.c	2005-04-26 16:48:12.000000000 -0500
@@ -230,7 +230,7 @@ ssize_t tpm_show_pcrs(struct device *dev
 	char *str = buf;
 
 	struct tpm_chip *chip =
-	    pci_get_drvdata(container_of(dev, struct pci_dev, dev));
+	    pci_get_drvdata(to_pci_dev(dev));
 	if (chip == NULL)
 		return -ENODEV;
 
@@ -273,7 +273,7 @@ ssize_t tpm_show_pubek(struct device *de
 	char *str = buf;
 
 	struct tpm_chip *chip =
-	    pci_get_drvdata(container_of(dev, struct pci_dev, dev));
+	    pci_get_drvdata(to_pci_dev(dev));
 	if (chip == NULL)
 		return -ENODEV;
 
@@ -352,7 +352,7 @@ ssize_t tpm_show_caps(struct device *dev
 	char *str = buf;
 
 	struct tpm_chip *chip =
-	    pci_get_drvdata(container_of(dev, struct pci_dev, dev));
+	    pci_get_drvdata(to_pci_dev(dev));
 	if (chip == NULL)
 		return -ENODEV;
 
\

Re: [PATCH] Add TPM hardware enablement driver

[Nish Aravamudan]

On Wed, 9 Mar 2005 16:42:01 -0800, Greg KH <greg@kroah.com> wrote:
> ChangeSet 1.2035, 2005/03/09 10:12:19-08:00, kjhall@us.ibm.com
> 
> [PATCH] Add TPM hardware enablement driver

<snip>

> +void tpm_time_expired(unsigned long ptr)
> +{
> +       int *exp = (int *) ptr;
> +       *exp = 1;
> +}
> +
> +EXPORT_SYMBOL_GPL(tpm_time_expired);

<snip>

> +       down(&chip->timer_manipulation_mutex);
> +       chip->time_expired = 0;
> +       init_timer(&chip->device_timer);
> +       chip->device_timer.function = tpm_time_expired;
> +       chip->device_timer.expires = jiffies + 2 * 60 * HZ;
> +       chip->device_timer.data = (unsigned long) &chip->time_expired;
> +       add_timer(&chip->device_timer);
> +       up(&chip->timer_manipulation_mutex);
> +
> +       do {
> +               u8 status = inb(chip->vendor->base + 1);
> +               if ((status & chip->vendor->req_complete_mask) ==
> +                   chip->vendor->req_complete_val) {
> +                       down(&chip->timer_manipulation_mutex);
> +                       del_singleshot_timer_sync(&chip->device_timer);
> +                       up(&chip->timer_manipulation_mutex);
> +                       goto out_recv;
> +               }
> +               set_current_state(TASK_UNINTERRUPTIBLE);
> +               schedule_timeout(TPM_TIMEOUT);
> +               rmb();
> +       } while (!chip->time_expired);

<snip>

It seems like this use of schedule_timeout() and the others are a bit
excessive. In this case, a timer is set to go off in 2 hours or so,
with tpm_time_expired() as the callback. tpm_time_expired(), it seems
just takes data and sets it to 1, which in this case is
chip->time_expired (and is similar in the other cases). We then loop
while (!chip->time_expired), which to me means until
chip->device_timer goes off, checking if the request is complete every
5 milliseconds. The chip->device_timer doesn't really do anything,
does it? It just guarantees a maximum time (of 2 hours). Couldn't the
same be achieved with (please excuse the lack of tabs, any real
patches I submit will have them):

unsigned long stop = jiffies + 2 * 60 * HZ;
do {
     u8 status = inb(chip->vendor->base + 1);
     if ((status & chip->vendor->req_complete_mask ==
           chip->vendor->req_complete_val)
               goto out_recv;
     msleep(TPM_TIMEOUT); // TPM_TIMEOUT could now be 5 ms
     rmb();
} while (time_before(jiffies, stop);

I think similar replacements would work in the other locations.

If people agree, I will send patches.

Thanks,
Nish

Re: [PATCH] Add TPM hardware enablement driver

[Nish Aravamudan]

On Thu, 10 Mar 2005 09:35:36 -0800, Nish Aravamudan
<nish.aravamudan@gmail.com> wrote:
> On Wed, 9 Mar 2005 16:42:01 -0800, Greg KH <greg@kroah.com> wrote:
> > ChangeSet 1.2035, 2005/03/09 10:12:19-08:00, kjhall@us.ibm.com
> >
> > [PATCH] Add TPM hardware enablement driver
> 
> <snip>
> 
> > +void tpm_time_expired(unsigned long ptr)
> > +{
> > +       int *exp = (int *) ptr;
> > +       *exp = 1;
> > +}
> > +
> > +EXPORT_SYMBOL_GPL(tpm_time_expired);
> 
> <snip>
> 
> > +       down(&chip->timer_manipulation_mutex);
> > +       chip->time_expired = 0;
> > +       init_timer(&chip->device_timer);
> > +       chip->device_timer.function = tpm_time_expired;
> > +       chip->device_timer.expires = jiffies + 2 * 60 * HZ;
> > +       chip->device_timer.data = (unsigned long) &chip->time_expired;
> > +       add_timer(&chip->device_timer);
> > +       up(&chip->timer_manipulation_mutex);
> > +
> > +       do {
> > +               u8 status = inb(chip->vendor->base + 1);
> > +               if ((status & chip->vendor->req_complete_mask) ==
> > +                   chip->vendor->req_complete_val) {
> > +                       down(&chip->timer_manipulation_mutex);
> > +                       del_singleshot_timer_sync(&chip->device_timer);
> > +                       up(&chip->timer_manipulation_mutex);
> > +                       goto out_recv;
> > +               }
> > +               set_current_state(TASK_UNINTERRUPTIBLE);
> > +               schedule_timeout(TPM_TIMEOUT);
> > +               rmb();
> > +       } while (!chip->time_expired);
> 
> <snip>
> 
> It seems like this use of schedule_timeout() and the others are a bit
> excessive. In this case, a timer is set to go off in 2 hours or so,
> with tpm_time_expired() as the callback. tpm_time_expired(), it seems
> just takes data and sets it to 1, which in this case is
> chip->time_expired (and is similar in the other cases). We then loop
> while (!chip->time_expired), which to me means until
> chip->device_timer goes off, checking if the request is complete every
> 5 milliseconds. The chip->device_timer doesn't really do anything,
> does it? It just guarantees a maximum time (of 2 hours). Couldn't the

Sorry for the slight exaggeration :) Not 2 hours, but 2 minutes :)
still, a long time.

-Nish

[PATCH] char/tpm: use msleep(), clean-up timers, fix typo

[Nishanth Aravamudan]

On Wed, Mar 09, 2005 at 04:42:01PM -0800, Greg KH wrote:
> ChangeSet 1.2035, 2005/03/09 10:12:19-08:00, kjhall@us.ibm.com
> 
> [PATCH] Add TPM hardware enablement driver
> 
> This patch is a device driver to enable new hardware.  The new hardware is
> the TPM chip as described by specifications at
> <http://www.trustedcomputinggroup.org>.  The TPM chip will enable you to
> use hardware to securely store and protect your keys and personal data.
> To use the chip according to the specification, you will need the Trusted
> Software Stack (TSS) of which an implementation for Linux is available at:
> <http://sourceforge.net/projects/trousers>.

Here is a patch that removes all callers of schedule_timeout() as I
previously mentioned:

Description: The TPM driver unnecessarily uses timers when it simply
needs to maintain a maximum delay via time_before(). msleep() is used
instead of schedule_timeout() to guarantee the task delays as expected.
While compile-testing, I found a typo in the driver, using tpm_chp
instead of tpm_chip. Remove the now unused timer callback function and
change TPM_TIMEOUT's units to milliseconds. Patch is compile-tested.

Signed-off-by: Nishanth Aravamudan <nacc@us.ibm.com>

diff -urpN 2.6.11-v/drivers/char/tpm/tpm.c 2.6.11/drivers/char/tpm/tpm.c
--- 2.6.11-v/drivers/char/tpm/tpm.c	2005-03-10 10:50:00.000000000 -0800
+++ 2.6.11/drivers/char/tpm/tpm.c	2005-03-10 11:00:50.000000000 -0800
@@ -19,7 +19,7 @@
  * 
  * Note, the TPM chip is not interrupt driven (only polling)
  * and can have very long timeouts (minutes!). Hence the unusual
- * calls to schedule_timeout.
+ * calls to msleep.
  *
  */
 
@@ -52,14 +52,6 @@ static void user_reader_timeout(unsigned
 	up(&chip->buffer_mutex);
 }
 
-void tpm_time_expired(unsigned long ptr)
-{
-	int *exp = (int *) ptr;
-	*exp = 1;
-}
-
-EXPORT_SYMBOL_GPL(tpm_time_expired);
-
 /*
  * Initialize the LPC bus and enable the TPM ports
  */
@@ -135,6 +127,7 @@ static ssize_t tpm_transmit(struct tpm_c
 	ssize_t len;
 	u32 count;
 	__be32 *native_size;
+	unsigned long stop;
 
 	native_size = (__force __be32 *) (buf + 2);
 	count = be32_to_cpu(*native_size);
@@ -155,28 +148,16 @@ static ssize_t tpm_transmit(struct tpm_c
 		return len;
 	}
 
-	down(&chip->timer_manipulation_mutex);
-	chip->time_expired = 0;
-	init_timer(&chip->device_timer);
-	chip->device_timer.function = tpm_time_expired;
-	chip->device_timer.expires = jiffies + 2 * 60 * HZ;
-	chip->device_timer.data = (unsigned long) &chip->time_expired;
-	add_timer(&chip->device_timer);
-	up(&chip->timer_manipulation_mutex);
-
+	stop = jiffies + 2 * 60 * HZ;
 	do {
 		u8 status = inb(chip->vendor->base + 1);
 		if ((status & chip->vendor->req_complete_mask) ==
 		    chip->vendor->req_complete_val) {
-			down(&chip->timer_manipulation_mutex);
-			del_singleshot_timer_sync(&chip->device_timer);
-			up(&chip->timer_manipulation_mutex);
 			goto out_recv;
 		}
-		set_current_state(TASK_UNINTERRUPTIBLE);
-		schedule_timeout(TPM_TIMEOUT);
+		msleep(TPM_TIMEOUT); /* CHECK */
 		rmb();
-	} while (!chip->time_expired);
+	} while (time_before(jiffies, stop));
 
 
 	chip->vendor->cancel(chip);
@@ -219,7 +200,7 @@ static ssize_t show_pcrs(struct device *
 	int i, j, index, num_pcrs;
 	char *str = buf;
 
-	struct tpm_chp *chip =
+	struct tpm_chip *chip =
 	    pci_get_drvdata(container_of(dev, struct pci_dev, dev));
 	if (chip == NULL)
 		return -ENODEV;
@@ -450,10 +431,8 @@ ssize_t tpm_write(struct file * file, co
 
 	/* cannot perform a write until the read has cleared
 	   either via tpm_read or a user_read_timer timeout */
-	while (atomic_read(&chip->data_pending) != 0) {
-		set_current_state(TASK_UNINTERRUPTIBLE);
-		schedule_timeout(TPM_TIMEOUT);
-	}
+	while (atomic_read(&chip->data_pending) != 0)
+		msleep(TPM_TIMEOUT);
 
 	down(&chip->buffer_mutex);
 
diff -urpN 2.6.11-v/drivers/char/tpm/tpm.h 2.6.11/drivers/char/tpm/tpm.h
--- 2.6.11-v/drivers/char/tpm/tpm.h	2005-03-10 10:50:00.000000000 -0800
+++ 2.6.11/drivers/char/tpm/tpm.h	2005-03-10 10:58:12.000000000 -0800
@@ -24,7 +24,7 @@
 #include <linux/delay.h>
 #include <linux/miscdevice.h>
 
-#define TPM_TIMEOUT msecs_to_jiffies(5)
+#define TPM_TIMEOUT	5	/* msecs */
 
 /* TPM addresses */
 #define	TPM_ADDR			0x4E
@@ -77,7 +77,6 @@ static inline void tpm_write_index(int i
 	outb(value & 0xFF, TPM_DATA);
 }
 
-extern void tpm_time_expired(unsigned long);
 extern int tpm_lpc_bus_init(struct pci_dev *, u16);
 
 extern int tpm_register_hardware(struct pci_dev *,
diff -urpN 2.6.11-v/drivers/char/tpm/tpm_nsc.c 2.6.11/drivers/char/tpm/tpm_nsc.c
--- 2.6.11-v/drivers/char/tpm/tpm_nsc.c	2005-03-10 10:50:00.000000000 -0800
+++ 2.6.11/drivers/char/tpm/tpm_nsc.c	2005-03-10 10:56:54.000000000 -0800
@@ -55,10 +55,7 @@
  */
 static int wait_for_stat(struct tpm_chip *chip, u8 mask, u8 val, u8 * data)
 {
-	int expired = 0;
-	struct timer_list status_timer =
-	    TIMER_INITIALIZER(tpm_time_expired, jiffies + 10 * HZ,
-			      (unsigned long) &expired);
+	unsigned long stop;
 
 	/* status immediately available check */
 	*data = inb(chip->vendor->base + NSC_STATUS);
@@ -66,17 +63,14 @@ static int wait_for_stat(struct tpm_chip
 		return 0;
 
 	/* wait for status */
-	add_timer(&status_timer);
+	stop = jiffies + 10 * HZ;
 	do {
-		set_current_state(TASK_UNINTERRUPTIBLE);
-		schedule_timeout(TPM_TIMEOUT);
+		msleep(TPM_TIMEOUT);
 		*data = inb(chip->vendor->base + 1);
-		if ((*data & mask) == val) {
-			del_singleshot_timer_sync(&status_timer);
+		if ((*data & mask) == val)
 			return 0;
-		}
 	}
-	while (!expired);
+	while (time_before(jiffies, stop));
 
 	return -EBUSY;
 }
@@ -84,10 +78,7 @@ static int wait_for_stat(struct tpm_chip
 static int nsc_wait_for_ready(struct tpm_chip *chip)
 {
 	int status;
-	int expired = 0;
-	struct timer_list status_timer =
-	    TIMER_INITIALIZER(tpm_time_expired, jiffies + 100,
-			      (unsigned long) &expired);
+	unsigned long stop;
 
 	/* status immediately available check */
 	status = inb(chip->vendor->base + NSC_STATUS);
@@ -97,19 +88,16 @@ static int nsc_wait_for_ready(struct tpm
 		return 0;
 
 	/* wait for status */
-	add_timer(&status_timer);
+	stop = jiffies + 100;
 	do {
-		set_current_state(TASK_UNINTERRUPTIBLE);
-		schedule_timeout(TPM_TIMEOUT);
+		msleep(TPM_TIMEOUT);
 		status = inb(chip->vendor->base + NSC_STATUS);
 		if (status & NSC_STATUS_OBF)
 			status = inb(chip->vendor->base + NSC_DATA);
-		if (status & NSC_STATUS_RDY) {
-			del_singleshot_timer_sync(&status_timer);
+		if (status & NSC_STATUS_RDY)
 			return 0;
-		}
 	}
-	while (!expired);
+	while (time_before(jiffies, stop));
 
 	dev_info(&chip->pci_dev->dev, "wait for ready failed\n");
 	return -EBUSY;

Re: [PATCH] Add TPM hardware enablement driver

[Alexey Dobriyan]

On Thursday 10 March 2005 02:42, Greg KH wrote:

> [PATCH] Add TPM hardware enablement driver

> +static ssize_t tpm_transmit(struct tpm_chip *chip, const char *buf,
> +			    size_t bufsiz)
> +{

> +	u32 count;
> +	__be32 *native_size;
> +
> +	native_size = (__force __be32 *) (buf + 2);
> +	count = be32_to_cpu(*native_size);

__force in a driver?

	count = be32_to_cpup((__be32 *) (buf + 2));

should be enough. Once done you can remove "native_size".

> +static int tpm_atml_recv(struct tpm_chip *chip, u8 * buf, size_t count)
> +{

> +	u32 size;

> +	__be32 *native_size;

> +	/* size of the data received */
> +	native_size = (__force __be32 *) (hdr + 2);
> +	size = be32_to_cpu(*native_size);

> +static int tpm_nsc_recv(struct tpm_chip *chip, u8 * buf, size_t count)
> +{

> +	u32 size;
> +	__be32 *native_size;

> +	native_size = (__force __be32 *) (buf + 2);
> +	size = be32_to_cpu(*native_size);

Same story.

	Alexey

[PATCH 9 of 12] Fix TPM driver -- remove unnecessary __force

[Kylene Hall]

On Thu, 10 Mar 2005, Alexey Dobriyan wrote:
> On Thursday 10 March 2005 02:42, Greg KH wrote:
> 
> > [PATCH] Add TPM hardware enablement driver
> 
> > +static ssize_t tpm_transmit(struct tpm_chip *chip, const char *buf,
> > +			    size_t bufsiz)
> > +{
> 
> > +	u32 count;
> > +	__be32 *native_size;
> > +
> > +	native_size = (__force __be32 *) (buf + 2);
> > +	count = be32_to_cpu(*native_size);
> 
> __force in a driver?
> 
> 	count = be32_to_cpup((__be32 *) (buf + 2));
> 
> should be enough. Once done you can remove "native_size".
> 
> > +static int tpm_atml_recv(struct tpm_chip *chip, u8 * buf, size_t count)
> > +{
> 
> > +	u32 size;
> 
> > +	__be32 *native_size;
> 
> > +	/* size of the data received */
> > +	native_size = (__force __be32 *) (hdr + 2);
> > +	size = be32_to_cpu(*native_size);
> 
> > +static int tpm_nsc_recv(struct tpm_chip *chip, u8 * buf, size_t count)
> > +{
> 
> > +	u32 size;
> > +	__be32 *native_size;
> 
> > +	native_size = (__force __be32 *) (buf + 2);
> > +	size = be32_to_cpu(*native_size);
> 
> Same story.
> 
> 	Alexey
> 
> 

The patch below removes the unnecessary use of __force.

Signed-off-by: Kylene Hall <kjhall@us.ibm.com>
---
diff -uprN --exclude='*.orig' linux-2.6.12-rc2/drivers/char/tpm/tpm.c linux-2.6.12-rc2-tpmdd/drivers/char/tpm/tpm.c
--- linux-2.6.12-rc2/drivers/char/tpm/tpm.c	2005-04-20 17:42:04.000000000 -0500
+++ linux-2.6.12-rc2-tpmdd/drivers/char/tpm/tpm.c	2005-04-20 17:49:06.000000000 -0500
@@ -142,11 +142,9 @@ static ssize_t tpm_transmit(struct tpm_c
 {
 	ssize_t len;
 	u32 count;
-	__be32 *native_size;
 	unsigned long stop;
 
-	native_size = (__force __be32 *) (buf + 2);
-	count = be32_to_cpu(*native_size);
+	count = be32_to_cpu(*((__be32 *) (buf + 2)));
 
 	if (count == 0)
 		return -ENODATA;
@@ -213,7 +211,8 @@ static ssize_t show_pcrs(struct device *
 {
 	u8 data[READ_PCR_RESULT_SIZE];
 	ssize_t len;
-	int i, j, index, num_pcrs;
+	int i, j, num_pcrs;
+	__be32 index;
 	char *str = buf;
 
 	struct tpm_chip *chip =
@@ -226,7 +225,7 @@ static ssize_t show_pcrs(struct device *
 	    < CAP_PCR_RESULT_SIZE)
 		return len;
 
-	num_pcrs = be32_to_cpu(*((__force __be32 *) (data + 14)));
+	num_pcrs = be32_to_cpu(*((__be32 *) (data + 14)));
 
 	for (i = 0; i < num_pcrs; i++) {
 		memcpy(data, pcrread, sizeof(pcrread));
@@ -256,8 +255,7 @@ static ssize_t show_pubek(struct device 
 {
 	u8 data[READ_PUBEK_RESULT_SIZE];
 	ssize_t len;
-	__be32 *native_val;
-	int i;
+	int i, rc;
 	char *str = buf;
 
 	struct tpm_chip *chip =
@@ -283,8 +281,6 @@ static ssize_t show_pubek(struct device 
 	   ignore checksum 20 bytes
 	 */
 
-	native_val = (__force __be32 *) (data + 34);
-
 	str +=
 	    sprintf(str,
 		    "Algorithm: %02X %02X %02X %02X\nEncscheme: %02X %02X\n"
@@ -295,8 +291,7 @@ static ssize_t show_pubek(struct device 
 		    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(*native_val)
-	    );
+		    be32_to_cpu(*((__be32 *) (data + 32))));
 
 	for (i = 0; i < 256; i++) {
 		str += sprintf(str, "%02X ", data[i + 39]);
@@ -345,7 +340,7 @@ static ssize_t show_caps(struct device *
 		return len;
 
 	str += sprintf(str, "Manufacturer: 0x%x\n",
-		       be32_to_cpu(*(data + 14)));
+		       be32_to_cpu(*((__be32 *) (data + 14))));
 
 	memcpy(data, cap_version, sizeof(cap_version));

[PATCH] char/tpm: use msleep(), clean-up timers, fix typo

[Nishanth Aravamudan]

Not sure what happened to the original mail, but I'm not seeing it
yet...

On Wed, Mar 09, 2005 at 04:42:01PM -0800, Greg KH wrote:
> ChangeSet 1.2035, 2005/03/09 10:12:19-08:00, kjhall@us.ibm.com
> 
> [PATCH] Add TPM hardware enablement driver
> 
> This patch is a device driver to enable new hardware.  The new hardware is
> the TPM chip as described by specifications at
> <http://www.trustedcomputinggroup.org>.  The TPM chip will enable you to
> use hardware to securely store and protect your keys and personal data.
> To use the chip according to the specification, you will need the Trusted
> Software Stack (TSS) of which an implementation for Linux is available at:
> <http://sourceforge.net/projects/trousers>.

Here is a patch that removes all callers of schedule_timeout() as I
previously mentioned:

Description: The TPM driver unnecessarily uses timers when it simply
needs to maintain a maximum delay via time_before(). msleep() is used
instead of schedule_timeout() to guarantee the task delays as expected.
While compile-testing, I found a typo in the driver, using tpm_chp
instead of tpm_chip. Remove the now unused timer callback function and
change TPM_TIMEOUT's units to milliseconds. Patch is compile-tested.

Signed-off-by: Nishanth Aravamudan <nacc@us.ibm.com>

diff -urpN 2.6.11-v/drivers/char/tpm/tpm.c 2.6.11/drivers/char/tpm/tpm.c
--- 2.6.11-v/drivers/char/tpm/tpm.c	2005-03-10 10:50:00.000000000 -0800
+++ 2.6.11/drivers/char/tpm/tpm.c	2005-03-10 11:00:50.000000000 -0800
@@ -19,7 +19,7 @@
  * 
  * Note, the TPM chip is not interrupt driven (only polling)
  * and can have very long timeouts (minutes!). Hence the unusual
- * calls to schedule_timeout.
+ * calls to msleep.
  *
  */
 
@@ -52,14 +52,6 @@ static void user_reader_timeout(unsigned
 	up(&chip->buffer_mutex);
 }
 
-void tpm_time_expired(unsigned long ptr)
-{
-	int *exp = (int *) ptr;
-	*exp = 1;
-}
-
-EXPORT_SYMBOL_GPL(tpm_time_expired);
-
 /*
  * Initialize the LPC bus and enable the TPM ports
  */
@@ -135,6 +127,7 @@ static ssize_t tpm_transmit(struct tpm_c
 	ssize_t len;
 	u32 count;
 	__be32 *native_size;
+	unsigned long stop;
 
 	native_size = (__force __be32 *) (buf + 2);
 	count = be32_to_cpu(*native_size);
@@ -155,28 +148,16 @@ static ssize_t tpm_transmit(struct tpm_c
 		return len;
 	}
 
-	down(&chip->timer_manipulation_mutex);
-	chip->time_expired = 0;
-	init_timer(&chip->device_timer);
-	chip->device_timer.function = tpm_time_expired;
-	chip->device_timer.expires = jiffies + 2 * 60 * HZ;
-	chip->device_timer.data = (unsigned long) &chip->time_expired;
-	add_timer(&chip->device_timer);
-	up(&chip->timer_manipulation_mutex);
-
+	stop = jiffies + 2 * 60 * HZ;
 	do {
 		u8 status = inb(chip->vendor->base + 1);
 		if ((status & chip->vendor->req_complete_mask) ==
 		    chip->vendor->req_complete_val) {
-			down(&chip->timer_manipulation_mutex);
-			del_singleshot_timer_sync(&chip->device_timer);
-			up(&chip->timer_manipulation_mutex);
 			goto out_recv;
 		}
-		set_current_state(TASK_UNINTERRUPTIBLE);
-		schedule_timeout(TPM_TIMEOUT);
+		msleep(TPM_TIMEOUT); /* CHECK */
 		rmb();
-	} while (!chip->time_expired);
+	} while (time_before(jiffies, stop));
 
 
 	chip->vendor->cancel(chip);
@@ -219,7 +200,7 @@ static ssize_t show_pcrs(struct device *
 	int i, j, index, num_pcrs;
 	char *str = buf;
 
-	struct tpm_chp *chip =
+	struct tpm_chip *chip =
 	    pci_get_drvdata(container_of(dev, struct pci_dev, dev));
 	if (chip == NULL)
 		return -ENODEV;
@@ -450,10 +431,8 @@ ssize_t tpm_write(struct file * file, co
 
 	/* cannot perform a write until the read has cleared
 	   either via tpm_read or a user_read_timer timeout */
-	while (atomic_read(&chip->data_pending) != 0) {
-		set_current_state(TASK_UNINTERRUPTIBLE);
-		schedule_timeout(TPM_TIMEOUT);
-	}
+	while (atomic_read(&chip->data_pending) != 0)
+		msleep(TPM_TIMEOUT);
 
 	down(&chip->buffer_mutex);
 
diff -urpN 2.6.11-v/drivers/char/tpm/tpm.h 2.6.11/drivers/char/tpm/tpm.h
--- 2.6.11-v/drivers/char/tpm/tpm.h	2005-03-10 10:50:00.000000000 -0800
+++ 2.6.11/drivers/char/tpm/tpm.h	2005-03-10 10:58:12.000000000 -0800
@@ -24,7 +24,7 @@
 #include <linux/delay.h>
 #include <linux/miscdevice.h>
 
-#define TPM_TIMEOUT msecs_to_jiffies(5)
+#define TPM_TIMEOUT	5	/* msecs */
 
 /* TPM addresses */
 #define	TPM_ADDR			0x4E
@@ -77,7 +77,6 @@ static inline void tpm_write_index(int i
 	outb(value & 0xFF, TPM_DATA);
 }
 
-extern void tpm_time_expired(unsigned long);
 extern int tpm_lpc_bus_init(struct pci_dev *, u16);
 
 extern int tpm_register_hardware(struct pci_dev *,
diff -urpN 2.6.11-v/drivers/char/tpm/tpm_nsc.c 2.6.11/drivers/char/tpm/tpm_nsc.c
--- 2.6.11-v/drivers/char/tpm/tpm_nsc.c	2005-03-10 10:50:00.000000000 -0800
+++ 2.6.11/drivers/char/tpm/tpm_nsc.c	2005-03-10 10:56:54.000000000 -0800
@@ -55,10 +55,7 @@
  */
 static int wait_for_stat(struct tpm_chip *chip, u8 mask, u8 val, u8 * data)
 {
-	int expired = 0;
-	struct timer_list status_timer =
-	    TIMER_INITIALIZER(tpm_time_expired, jiffies + 10 * HZ,
-			      (unsigned long) &expired);
+	unsigned long stop;
 
 	/* status immediately available check */
 	*data = inb(chip->vendor->base + NSC_STATUS);
@@ -66,17 +63,14 @@ static int wait_for_stat(struct tpm_chip
 		return 0;
 
 	/* wait for status */
-	add_timer(&status_timer);
+	stop = jiffies + 10 * HZ;
 	do {
-		set_current_state(TASK_UNINTERRUPTIBLE);
-		schedule_timeout(TPM_TIMEOUT);
+		msleep(TPM_TIMEOUT);
 		*data = inb(chip->vendor->base + 1);
-		if ((*data & mask) == val) {
-			del_singleshot_timer_sync(&status_timer);
+		if ((*data & mask) == val)
 			return 0;
-		}
 	}
-	while (!expired);
+	while (time_before(jiffies, stop));
 
 	return -EBUSY;
 }
@@ -84,10 +78,7 @@ static int wait_for_stat(struct tpm_chip
 static int nsc_wait_for_ready(struct tpm_chip *chip)
 {
 	int status;
-	int expired = 0;
-	struct timer_list status_timer =
-	    TIMER_INITIALIZER(tpm_time_expired, jiffies + 100,
-			      (unsigned long) &expired);
+	unsigned long stop;
 
 	/* status immediately available check */
 	status = inb(chip->vendor->base + NSC_STATUS);
@@ -97,19 +88,16 @@ static int nsc_wait_for_ready(struct tpm
 		return 0;
 
 	/* wait for status */
-	add_timer(&status_timer);
+	stop = jiffies + 100;
 	do {
-		set_current_state(TASK_UNINTERRUPTIBLE);
-		schedule_timeout(TPM_TIMEOUT);
+		msleep(TPM_TIMEOUT);
 		status = inb(chip->vendor->base + NSC_STATUS);
 		if (status & NSC_STATUS_OBF)
 			status = inb(chip->vendor->base + NSC_DATA);
-		if (status & NSC_STATUS_RDY) {
-			del_singleshot_timer_sync(&status_timer);
+		if (status & NSC_STATUS_RDY)
 			return 0;
-		}
 	}
-	while (!expired);
+	while (time_before(jiffies, stop));
 
 	dev_info(&chip->pci_dev->dev, "wait for ready failed\n");
 	return -EBUSY;

Re: [PATCH] char/tpm: use msleep(), clean-up timers, fix typo

[Kylene Hall]

I have tested this patch and agree that using msleep is the right.  Please 
apply this patch to the tpm driver.  One hunk might fail b/c the 
typo has been fixed already.

Thanks,
Kylie Hall

On Fri, 11 Mar 2005, Nishanth Aravamudan wrote:

> Not sure what happened to the original mail, but I'm not seeing it
> yet...
> 
> On Wed, Mar 09, 2005 at 04:42:01PM -0800, Greg KH wrote:
> > ChangeSet 1.2035, 2005/03/09 10:12:19-08:00, kjhall@us.ibm.com
> > 
> > [PATCH] Add TPM hardware enablement driver
> > 
> > This patch is a device driver to enable new hardware.  The new hardware is
> > the TPM chip as described by specifications at
> > <http://www.trustedcomputinggroup.org>.  The TPM chip will enable you to
> > use hardware to securely store and protect your keys and personal data.
> > To use the chip according to the specification, you will need the Trusted
> > Software Stack (TSS) of which an implementation for Linux is available at:
> > <http://sourceforge.net/projects/trousers>.
> 
> Here is a patch that removes all callers of schedule_timeout() as I
> previously mentioned:
> 
> Description: The TPM driver unnecessarily uses timers when it simply
> needs to maintain a maximum delay via time_before(). msleep() is used
> instead of schedule_timeout() to guarantee the task delays as expected.
> While compile-testing, I found a typo in the driver, using tpm_chp
> instead of tpm_chip. Remove the now unused timer callback function and
> change TPM_TIMEOUT's units to milliseconds. Patch is compile-tested.
> 
> Signed-off-by: Nishanth Aravamudan <nacc@us.ibm.com>
> 
> diff -urpN 2.6.11-v/drivers/char/tpm/tpm.c 2.6.11/drivers/char/tpm/tpm.c
> --- 2.6.11-v/drivers/char/tpm/tpm.c	2005-03-10 10:50:00.000000000 -0800
> +++ 2.6.11/drivers/char/tpm/tpm.c	2005-03-10 11:00:50.000000000 -0800
> @@ -19,7 +19,7 @@
>   * 
>   * Note, the TPM chip is not interrupt driven (only polling)
>   * and can have very long timeouts (minutes!). Hence the unusual
> - * calls to schedule_timeout.
> + * calls to msleep.
>   *
>   */
>  
> @@ -52,14 +52,6 @@ static void user_reader_timeout(unsigned
>  	up(&chip->buffer_mutex);
>  }
>  
> -void tpm_time_expired(unsigned long ptr)
> -{
> -	int *exp = (int *) ptr;
> -	*exp = 1;
> -}
> -
> -EXPORT_SYMBOL_GPL(tpm_time_expired);
> -
>  /*
>   * Initialize the LPC bus and enable the TPM ports
>   */
> @@ -135,6 +127,7 @@ static ssize_t tpm_transmit(struct tpm_c
>  	ssize_t len;
>  	u32 count;
>  	__be32 *native_size;
> +	unsigned long stop;
>  
>  	native_size = (__force __be32 *) (buf + 2);
>  	count = be32_to_cpu(*native_size);
> @@ -155,28 +148,16 @@ static ssize_t tpm_transmit(struct tpm_c
>  		return len;
>  	}
>  
> -	down(&chip->timer_manipulation_mutex);
> -	chip->time_expired = 0;
> -	init_timer(&chip->device_timer);
> -	chip->device_timer.function = tpm_time_expired;
> -	chip->device_timer.expires = jiffies + 2 * 60 * HZ;
> -	chip->device_timer.data = (unsigned long) &chip->time_expired;
> -	add_timer(&chip->device_timer);
> -	up(&chip->timer_manipulation_mutex);
> -
> +	stop = jiffies + 2 * 60 * HZ;
>  	do {
>  		u8 status = inb(chip->vendor->base + 1);
>  		if ((status & chip->vendor->req_complete_mask) ==
>  		    chip->vendor->req_complete_val) {
> -			down(&chip->timer_manipulation_mutex);
> -			del_singleshot_timer_sync(&chip->device_timer);
> -			up(&chip->timer_manipulation_mutex);
>  			goto out_recv;
>  		}
> -		set_current_state(TASK_UNINTERRUPTIBLE);
> -		schedule_timeout(TPM_TIMEOUT);
> +		msleep(TPM_TIMEOUT); /* CHECK */
>  		rmb();
> -	} while (!chip->time_expired);
> +	} while (time_before(jiffies, stop));
>  
>  
>  	chip->vendor->cancel(chip);
> @@ -219,7 +200,7 @@ static ssize_t show_pcrs(struct device *
>  	int i, j, index, num_pcrs;
>  	char *str = buf;
>  
> -	struct tpm_chp *chip =
> +	struct tpm_chip *chip =
>  	    pci_get_drvdata(container_of(dev, struct pci_dev, dev));
>  	if (chip == NULL)
>  		return -ENODEV;
> @@ -450,10 +431,8 @@ ssize_t tpm_write(struct file * file, co
>  
>  	/* cannot perform a write until the read has cleared
>  	   either via tpm_read or a user_read_timer timeout */
> -	while (atomic_read(&chip->data_pending) != 0) {
> -		set_current_state(TASK_UNINTERRUPTIBLE);
> -		schedule_timeout(TPM_TIMEOUT);
> -	}
> +	while (atomic_read(&chip->data_pending) != 0)
> +		msleep(TPM_TIMEOUT);
>  
>  	down(&chip->buffer_mutex);
>  
> diff -urpN 2.6.11-v/drivers/char/tpm/tpm.h 2.6.11/drivers/char/tpm/tpm.h
> --- 2.6.11-v/drivers/char/tpm/tpm.h	2005-03-10 10:50:00.000000000 -0800
> +++ 2.6.11/drivers/char/tpm/tpm.h	2005-03-10 10:58:12.000000000 -0800
> @@ -24,7 +24,7 @@
>  #include <linux/delay.h>
>  #include <linux/miscdevice.h>
>  
> -#define TPM_TIMEOUT msecs_to_jiffies(5)
> +#define TPM_TIMEOUT	5	/* msecs */
>  
>  /* TPM addresses */
>  #define	TPM_ADDR			0x4E
> @@ -77,7 +77,6 @@ static inline void tpm_write_index(int i
>  	outb(value & 0xFF, TPM_DATA);
>  }
>  
> -extern void tpm_time_expired(unsigned long);
>  extern int tpm_lpc_bus_init(struct pci_dev *, u16);
>  
>  extern int tpm_register_hardware(struct pci_dev *,
> diff -urpN 2.6.11-v/drivers/char/tpm/tpm_nsc.c 2.6.11/drivers/char/tpm/tpm_nsc.c
> --- 2.6.11-v/drivers/char/tpm/tpm_nsc.c	2005-03-10 10:50:00.000000000 -0800
> +++ 2.6.11/drivers/char/tpm/tpm_nsc.c	2005-03-10 10:56:54.000000000 -0800
> @@ -55,10 +55,7 @@
>   */
>  static int wait_for_stat(struct tpm_chip *chip, u8 mask, u8 val, u8 * data)
>  {
> -	int expired = 0;
> -	struct timer_list status_timer =
> -	    TIMER_INITIALIZER(tpm_time_expired, jiffies + 10 * HZ,
> -			      (unsigned long) &expired);
> +	unsigned long stop;
>  
>  	/* status immediately available check */
>  	*data = inb(chip->vendor->base + NSC_STATUS);
> @@ -66,17 +63,14 @@ static int wait_for_stat(struct tpm_chip
>  		return 0;
>  
>  	/* wait for status */
> -	add_timer(&status_timer);
> +	stop = jiffies + 10 * HZ;
>  	do {
> -		set_current_state(TASK_UNINTERRUPTIBLE);
> -		schedule_timeout(TPM_TIMEOUT);
> +		msleep(TPM_TIMEOUT);
>  		*data = inb(chip->vendor->base + 1);
> -		if ((*data & mask) == val) {
> -			del_singleshot_timer_sync(&status_timer);
> +		if ((*data & mask) == val)
>  			return 0;
> -		}
>  	}
> -	while (!expired);
> +	while (time_before(jiffies, stop));
>  
>  	return -EBUSY;
>  }
> @@ -84,10 +78,7 @@ static int wait_for_stat(struct tpm_chip
>  static int nsc_wait_for_ready(struct tpm_chip *chip)
>  {
>  	int status;
> -	int expired = 0;
> -	struct timer_list status_timer =
> -	    TIMER_INITIALIZER(tpm_time_expired, jiffies + 100,
> -			      (unsigned long) &expired);
> +	unsigned long stop;
>  
>  	/* status immediately available check */
>  	status = inb(chip->vendor->base + NSC_STATUS);
> @@ -97,19 +88,16 @@ static int nsc_wait_for_ready(struct tpm
>  		return 0;
>  
>  	/* wait for status */
> -	add_timer(&status_timer);
> +	stop = jiffies + 100;
>  	do {
> -		set_current_state(TASK_UNINTERRUPTIBLE);
> -		schedule_timeout(TPM_TIMEOUT);
> +		msleep(TPM_TIMEOUT);
>  		status = inb(chip->vendor->base + NSC_STATUS);
>  		if (status & NSC_STATUS_OBF)
>  			status = inb(chip->vendor->base + NSC_DATA);
> -		if (status & NSC_STATUS_RDY) {
> -			del_singleshot_timer_sync(&status_timer);
> +		if (status & NSC_STATUS_RDY)
>  			return 0;
> -		}
>  	}
> -	while (!expired);
> +	while (time_before(jiffies, stop));
>  
>  	dev_info(&chip->pci_dev->dev, "wait for ready failed\n");
>  	return -EBUSY;
> 
> 

Re: [PATCH] char/tpm: use msleep(), clean-up timers, fix typo

[Nish Aravamudan]

On 4/15/05, Kylene Hall <kjhall@us.ibm.com> wrote:
> I have tested this patch and agree that using msleep is the right.  Please
> apply this patch to the tpm driver.  One hunk might fail b/c the
> typo has been fixed already.

Would you like me to respin the patch, Greg? Or is the failed hunk ok?

Thanks,
Nish

Re: [PATCH] char/tpm: use msleep(), clean-up timers, fix typo

[Greg KH]

On Fri, Apr 15, 2005 at 01:44:55PM -0700, Nish Aravamudan wrote:
> On 4/15/05, Kylene Hall <kjhall@us.ibm.com> wrote:
> > I have tested this patch and agree that using msleep is the right.  Please
> > apply this patch to the tpm driver.  One hunk might fail b/c the
> > typo has been fixed already.
> 
> Would you like me to respin the patch, Greg? Or is the failed hunk ok?

I'm sorry, but I am not in charge of accepting patches for the tpm
driver.  Why not go through the listed maintainer for this process, they
should know how to get it into the mainline kernel tree properly.  If
not, why would they be listed as the maintainer?  :)

thanks,

greg k-h

Re: [PATCH] char/tpm: use msleep(), clean-up timers, fix typo

[Nish Aravamudan]

On 4/15/05, Greg KH <greg@kroah.com> wrote:
> On Fri, Apr 15, 2005 at 01:44:55PM -0700, Nish Aravamudan wrote:
> > On 4/15/05, Kylene Hall <kjhall@us.ibm.com> wrote:
> > > I have tested this patch and agree that using msleep is the right.  Please
> > > apply this patch to the tpm driver.  One hunk might fail b/c the
> > > typo has been fixed already.
> >
> > Would you like me to respin the patch, Greg? Or is the failed hunk ok?
> 
> I'm sorry, but I am not in charge of accepting patches for the tpm
> driver.  Why not go through the listed maintainer for this process, they
> should know how to get it into the mainline kernel tree properly.  If
> not, why would they be listed as the maintainer?  :)

Sorry about that Greg, I was just mixed-up since you had pushed the
original patch in; my fault. Will remove you from my other reply.

Thanks,
Nish

Re: [PATCH] char/tpm: use msleep(), clean-up timers, fix typo

[Nish Aravamudan]

On 4/15/05, Greg KH <greg@kroah.com> wrote:
> On Fri, Apr 15, 2005 at 01:44:55PM -0700, Nish Aravamudan wrote:
> > On 4/15/05, Kylene Hall <kjhall@us.ibm.com> wrote:
> > > I have tested this patch and agree that using msleep is the right.  Please
> > > apply this patch to the tpm driver.  One hunk might fail b/c the
> > > typo has been fixed already.
> >
> > Would you like me to respin the patch, Greg? Or is the failed hunk ok?
> 
> I'm sorry, but I am not in charge of accepting patches for the tpm
> driver.  Why not go through the listed maintainer for this process, they
> should know how to get it into the mainline kernel tree properly.  If
> not, why would they be listed as the maintainer?  :)

Kylene, there is no entry in MAINTAINERS as of 2.6.12-rc2 for the TPM
driver? Should there be?

I am assuming tpmdd_devel can take care of merging the patch and
pushing to mainline? Please do so.

Thanks,
Nish