2005-03-10 01:10:45

by Greg KH

[permalink] [raw]
Subject: [BK PATCH] Add TPM driver support for 2.6.11

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


2005-03-10 01:11:53

by Greg KH

[permalink] [raw]
Subject: [PATCH] tpm: fix cause of SMP stack traces

ChangeSet 1.2036, 2005/03/09 10:12:38-08:00, [email protected]

[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
<[email protected]>

Signed-off-by: Kylene Hall <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>


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;
}


2005-03-10 01:15:59

by Greg KH

[permalink] [raw]
Subject: [PATCH] tpm_msc-build-fix

ChangeSet 1.2037, 2005/03/09 10:12:56-08:00, [email protected]

[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 <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>


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,

2005-03-10 01:10:45

by Greg KH

[permalink] [raw]
Subject: [PATCH] Add TPM hardware enablement driver

ChangeSet 1.2035, 2005/03/09 10:12:19-08:00, [email protected]

[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 <[email protected]>
Signed-off-by: Reiner Sailer <[email protected]>
Signed-off-by: Dave Safford <[email protected]>
Signed-off-by: Kylene Hall <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>


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 <[email protected]>
+ * Dave Safford <[email protected]>
+ * Reiner Sailer <[email protected]>
+ * Kylene Hall <[email protected]>
+ *
+ * Maintained by: <[email protected]>
+ *
+ * Device driver for TCG/TCPA TPM (trusted platform module).
+ * Specifications at http://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 ([email protected])");
+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 <[email protected]>
+ * Dave Safford <[email protected]>
+ * Reiner Sailer <[email protected]>
+ * Kylene Hall <[email protected]>
+ *
+ * Maintained by: <[email protected]>
+ *
+ * Device driver for TCG/TCPA TPM (trusted platform module).
+ * Specifications at http://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 <[email protected]>
+ * Dave Safford <[email protected]>
+ * Reiner Sailer <[email protected]>
+ * Kylene Hall <[email protected]>
+ *
+ * Maintained by: <[email protected]>
+ *
+ * Device driver for TCG/TCPA TPM (trusted platform module).
+ * Specifications at http://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 ([email protected])");
+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 <[email protected]>
+ * Dave Safford <[email protected]>
+ * Reiner Sailer <[email protected]>
+ * Kylene Hall <[email protected]>
+ *
+ * Maintained by: <[email protected]>
+ *
+ * Device driver for TCG/TCPA TPM (trusted platform module).
+ * Specifications at http://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 ([email protected])");
+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

2005-03-10 01:10:46

by Greg KH

[permalink] [raw]
Subject: [PATCH] tpm-build-fix

ChangeSet 1.2039, 2005/03/09 10:13:34-08:00, [email protected]

[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 <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>


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;

2005-03-10 03:24:37

by Greg KH

[permalink] [raw]
Subject: [PATCH] tpm_atmel build fix

ChangeSet 1.2038, 2005/03/09 10:13:15-08:00, [email protected]

[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 <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>


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,

2005-03-10 04:08:44

by Jeff Garzik

[permalink] [raw]
Subject: Re: [PATCH] Add TPM hardware enablement driver

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 <[email protected]>
> + * Dave Safford <[email protected]>
> + * Reiner Sailer <[email protected]>
> + * Kylene Hall <[email protected]>
> + *
> + * Maintained by: <[email protected]>
> + *
> + * Device driver for TCG/TCPA TPM (trusted platform module).
> + * Specifications at http://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 ([email protected])");
> +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 <[email protected]>
> + * Dave Safford <[email protected]>
> + * Reiner Sailer <[email protected]>
> + * Kylene Hall <[email protected]>
> + *
> + * Maintained by: <[email protected]>
> + *
> + * Device driver for TCG/TCPA TPM (trusted platform module).
> + * Specifications at http://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 <[email protected]>
> + * Dave Safford <[email protected]>
> + * Reiner Sailer <[email protected]>
> + * Kylene Hall <[email protected]>
> + *
> + * Maintained by: <[email protected]>
> + *
> + * Device driver for TCG/TCPA TPM (trusted platform module).
> + * Specifications at http://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 ([email protected])");
> +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 <[email protected]>
> + * Dave Safford <[email protected]>
> + * Reiner Sailer <[email protected]>
> + * Kylene Hall <[email protected]>
> + *
> + * Maintained by: <[email protected]>
> + *
> + * Device driver for TCG/TCPA TPM (trusted platform module).
> + * Specifications at http://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?

2005-03-10 17:41:27

by Nish Aravamudan

[permalink] [raw]
Subject: Re: [PATCH] Add TPM hardware enablement driver

On Wed, 9 Mar 2005 16:42:01 -0800, Greg KH <[email protected]> wrote:
> ChangeSet 1.2035, 2005/03/09 10:12:19-08:00, [email protected]
>
> [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

2005-03-10 18:31:19

by Nish Aravamudan

[permalink] [raw]
Subject: Re: [PATCH] Add TPM hardware enablement driver

On Thu, 10 Mar 2005 09:35:36 -0800, Nish Aravamudan
<[email protected]> wrote:
> On Wed, 9 Mar 2005 16:42:01 -0800, Greg KH <[email protected]> wrote:
> > ChangeSet 1.2035, 2005/03/09 10:12:19-08:00, [email protected]
> >
> > [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

2005-03-10 19:16:16

by Nishanth Aravamudan

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

On Wed, Mar 09, 2005 at 04:42:01PM -0800, Greg KH wrote:
> ChangeSet 1.2035, 2005/03/09 10:12:19-08:00, [email protected]
>
> [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 <[email protected]>

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;

2005-03-10 20:11:42

by Alexey Dobriyan

[permalink] [raw]
Subject: Re: [PATCH] Add TPM hardware enablement driver

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

2005-03-11 18:49:51

by Nishanth Aravamudan

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

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, [email protected]
>
> [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 <[email protected]>

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;

2005-03-16 00:02:59

by Kylene Jo Hall

[permalink] [raw]
Subject: Re: [PATCH] Add TPM hardware enablement driver

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

2005-03-17 00:48:46

by Kylene Jo Hall

[permalink] [raw]
Subject: Re: [PATCH] Add TPM hardware enablement driver

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 <[email protected]>

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 ([email protected])");
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: [email protected]
L: [email protected]
S: Maintained

+TPM DEVICE DRIVER
+P: Kylene Hall
+M: [email protected]
+W: http://tpmdd.sourceforge.net
+L: [email protected]
+S: Maintained
+
UltraSPARC (sparc64):
P: David S. Miller
M: [email protected]

2005-03-23 02:02:48

by Jeff Garzik

[permalink] [raw]
Subject: Re: [PATCH] Add TPM hardware enablement driver

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


2005-03-24 07:07:25

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH] Add TPM hardware enablement driver

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

2005-03-24 21:05:11

by Jeff Garzik

[permalink] [raw]
Subject: Re: [PATCH] Add TPM hardware enablement driver

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


2005-03-24 21:33:20

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH] Add TPM hardware enablement driver

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

2005-04-05 16:15:58

by Kylene Jo Hall

[permalink] [raw]
Subject: Re: [PATCH] Add TPM hardware enablement driver

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


2005-04-08 20:08:08

by Kylene Jo Hall

[permalink] [raw]
Subject: Re: [PATCH] Add TPM hardware enablement driver

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
>



2005-04-09 08:32:07

by Ian Campbell

[permalink] [raw]
Subject: Re: [PATCH] Add TPM hardware enablement driver

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.


Attachments:
signature.asc (189.00 B)
This is a digitally signed message part

2005-04-15 20:24:43

by Kylene Jo Hall

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

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, [email protected]
> >
> > [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 <[email protected]>
>
> 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;
>
>

2005-04-15 20:44:57

by Nish Aravamudan

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

On 4/15/05, Kylene Hall <[email protected]> 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

2005-04-15 21:04:38

by Greg KH

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

On Fri, Apr 15, 2005 at 01:44:55PM -0700, Nish Aravamudan wrote:
> On 4/15/05, Kylene Hall <[email protected]> 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

2005-04-15 21:47:18

by Nish Aravamudan

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

On 4/15/05, Greg KH <[email protected]> wrote:
> On Fri, Apr 15, 2005 at 01:44:55PM -0700, Nish Aravamudan wrote:
> > On 4/15/05, Kylene Hall <[email protected]> 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

2005-04-15 21:50:21

by Nish Aravamudan

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

On 4/15/05, Greg KH <[email protected]> wrote:
> On Fri, Apr 15, 2005 at 01:44:55PM -0700, Nish Aravamudan wrote:
> > On 4/15/05, Kylene Hall <[email protected]> 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

2005-04-27 22:16:30

by Kylene Jo Hall

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

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 <[email protected]>
---
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;
}

2005-04-27 22:16:44

by Kylene Jo Hall

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

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 <[email protected]>
---
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 */

2005-04-27 22:18:16

by Kylene Jo Hall

[permalink] [raw]
Subject: [PATCH 4 of 12] Fix TPM driver -- read return code issue

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 <[email protected]>
---
--- 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);
}

2005-04-27 22:18:16

by Kylene Jo Hall

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

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 <[email protected]>
---
--- 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 ([email protected])");
MODULE_DESCRIPTION("TPM Driver");
MODULE_VERSION("2.0");

2005-04-27 22:22:39

by Kylene Jo Hall

[permalink] [raw]
Subject: [PATCH 9 of 12] Fix TPM driver -- remove unnecessary __force

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 <[email protected]>
---
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));

2005-04-27 22:22:40

by Kylene Jo Hall

[permalink] [raw]
Subject: [PATCH 7 of 12] Fix TPM driver -- use to_pci_dev

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 <[email protected]>
---
--- 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;

\

2005-04-27 22:28:13

by Greg KH

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

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

2005-04-27 22:26:59

by Kylene Jo Hall

[permalink] [raw]
Subject: [PATCH 6 of 12] Fix TPM driver -- how timer is initialized

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 <[email protected]>
---
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;

2005-04-27 22:19:49

by Kylene Jo Hall

[permalink] [raw]
Subject: [PATCH 5 of 12] Fix TPM driver -- large stack objects

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 <[email protected]>
---
--- 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;