tpm: Support for new chip type
PATCH 1/1 tpm: Support for new chip type
Hello LKML,
after some corrections here is a newer patch supporting the Infineon Trusted Platform Module SLD 9630 (TPM 1.1b), which is embedded on Intel-mainboards or in HP/Fujitsu-Siemens/Toshiba-Notebooks.
The module fits the interfaces created by IBM and was patched against the latest kernel-snapshot 2.6.13-rc2.
Further information about this module and a list of supported hardware can be found here: http://www.prosec.rub.de/tpm
Best Regards,
Marcel Selhorst
---
drivers/char/tpm/Kconfig +13 -0
@@ -35,5 +35,18 @@ config TCG_ATMEL will be accessible from within Linux. To compile this driver as a module, choose M here; the module will be called tpm_atmel. +config TCG_INFINEON + tristate "Infineon Technologies SLD 9630 TPM Interface" + depends on TCG_TPM + ---help--- + If you have a TPM security chip from Infineon Technologies + 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_infineon. + Further information on this driver and the supported hardware + can be found at http://www.prosec.rub.de/tpm + Note: To get debugging output into the module, you have to enable + CONFIG_DEBUG_KERNEL, CONFIG_DEBUG_INFO and CONFIG_PCI_DEBUG + endmenu
drivers/char/tpm/Makefile +1 -0
@@ -4,4 +4,5 @@ obj-$(CONFIG_TCG_TPM) += tpm.o obj-$(CONFIG_TCG_NSC) += tpm_nsc.o obj-$(CONFIG_TCG_ATMEL) += tpm_atmel.o +obj-$(CONFIG_TCG_INFINEON) += tpm_infineon.o
drivers/char/tpm/tpm_infineon.c +505 -0
505 lines not on this page · raw message
Hello!
Quoted 1 line · Marcel Selhorst
+++
linux/drivers/char/tpm/tpm_infineon.c2005-07-07 14:56:27.000000000 +0000
...
Quoted 1 line
+ * Specifications at www.trustedcomputinggroup.org
Blanks at end of line
drivers/char/tpm/tpm_infineon.c line 31
Quoted 8 lines
drivers/char/tpm/tpm_infineon.c +8
-0+/* Infineon specific delay definitions */ +enum infineon_tpm_delay { +TPM_MAX_WTX_PACKAGES =50 ,/* maximum number of WTX-packages */ +TPM_WTX_MSLEEP_TIME =20 ,/* msleep-Time for WTX-packages */ +TPM_MSLEEP_TIME =3 ,/* msleep-Time --> Interval to check status register */ +TPM_MAX_TRIES =5000 /* gives number of max. msleep()-calls before +throwing timeout */ +};
These look like unrelated constants, perhaps #define would be nicer?
drivers/char/tpm/tpm_infineon.c line 137
Quoted 3 lines
drivers/char/tpm/tpm_infineon.c +3
-0+static int wait (struct tpm_chip *chip,int wait_for_bit) +{ +
blank line
Review
Quoted 2 lines
(quoted diff) +2
-0+} +static void tpm_inf_cancel(struct tpm_chip *chip)
missing blank line
drivers/char/tpm/tpm_infineon.c line 378
Quoted 4 lines
drivers/char/tpm/tpm_infineon.c +4
-0+static int __init tpm_inf_probe (struct pci_dev *pci_dev, +const struct pci_device_id *pci_id) +{ +
blank lines
drivers/char/tpm/tpm_infineon.c line 459
Quoted 3 lines
drivers/char/tpm/tpm_infineon.c +3
-0+return 0 ; + + }else {
BYtE
Philipp
Signature · 3 lines
/ / (_)__ __ ____ __ Philipp Hahn / /__/ / _ \/ // /\ \/ / /____/_/_//_/\_,_/ /_/\_\ [email protected]
Quoted 22 lines · Marcel Selhorst
after some corrections here is a newer patch supporting the Infineon Trusted Platform Module SLD 9630 (TPM 1.1b), which is embedded on Intel-mainboards or in HP/Fujitsu-Siemens/Toshiba-Notebooks.
drivers/char/tpm/tpm_infineon.c +15
-0+static int empty_fifo (struct tpm_chip *chip,int clear_wrfifo) +{ +int status; +int check =0 ; +int i; +int j =0 ; + +if (clear_wrfifo) { +for (i =0 ; i <4096 ; i++) { + status =inb (chip->vendor->base +WRFIFO ); +if (status ==0xff ) { +if (check ==5 ) +break ; +else + check++;
drivers/char/tpm/tpm_infineon.c line 123
Quoted 10 lines
drivers/char/tpm/tpm_infineon.c +10
-0+ } + } + } +/* Note: The values which are currently in the FIFO of the TPM +are thrown away since there is no usage for them. Usually, +this has nothing to say, since the TPM will give its answer immediately +or will be aborted anyway, so the data here is usually garbage and useless. +We have to clean this, because the next communication with the TPM would +be rubbish, if there is still some old data in the Read FIFO. +*/
Could you reformat it a little to fit in 80 columns?
drivers/char/tpm/tpm_infineon.c line 124
Quoted 1 line
drivers/char/tpm/tpm_infineon.c +1
-0+ i =0 ;
i is used only in clear_wrfifo loop. Or sacrifice j instead.
Quoted 4 lines
drivers/char/tpm/tpm_infineon.c +4
-0+ do { + status = inb(chip->vendor->base + RDFIFO); + status = inb(chip->vendor->base + STAT); + j++;
drivers/char/tpm/tpm_infineon.c line 146
Quoted 10 lines
drivers/char/tpm/tpm_infineon.c +8
-0+if (j ==TPM_MAX_TRIES ) +return -EIO ; + }while ((status & (1 <<STAT_RDA )) !=0 ); +return 0 ; +} +static int wait (struct tpm_chip *chip,int wait_for_bit) +{ +if (i ==TPM_MAX_TRIES ) {/* when timeout occurs, print information and return -EIO */
We see dev_err() calls, we see -EIO. So only "timeout occurs" should be left.
Quoted 5 lines
drivers/char/tpm/tpm_infineon.c +5
-0+dev_err (&chip->pci_dev->dev,"Timeout in wait(" ); +if (wait_for_bit ==STAT_XFE ) +dev_err (&chip->pci_dev->dev,"STAT_XFE)\n" ); +if (wait_for_bit ==STAT_RDA ) +dev_err (&chip->pci_dev->dev,"STAT_RDA)\n" );
drivers/char/tpm/tpm_infineon.c line 169
Quoted 10 lines
drivers/char/tpm/tpm_infineon.c +9
-0+return -EIO ; + } +/* Note: WTX means Waiting-Time-Extension. Whenever the TPM +needs more calculation time, it sends a WTX-package, which has to +be acknowledged or aborted. This usually occurs if you are hammering +the TPM with key creation. Set the maximum number of WTX-packages in +the definitions above, if the number is reached, the waiting-time will be denied +and the TPM command has to be resend. +*/
Be consistent with the rest of the driver:
/* This style is OK. */
drivers/char/tpm/tpm_infineon.c line 200
Quoted 4 lines
drivers/char/tpm/tpm_infineon.c +3
-0+static int tpm_inf_recv (struct tpm_chip *chip,u8 * buf,size_t count) +{ + recv_begin:
Labels are placed at column zero usually.
drivers/char/tpm/tpm_infineon.c line 212
Quoted 11 lines
drivers/char/tpm/tpm_infineon.c +11
-0+dev_dbg (&chip->pci_dev->dev,"Receiving header: " ); + +for (i =0 ; i <4 ; i++) { + ret =wait (chip,STAT_RDA ); +if (ret) +return -EIO ; + + buf[i] =inb (chip->vendor->base +RDFIFO ); +dev_dbg (&chip->pci_dev->dev,"%02x " , buf[i]); + } +dev_dbg (&chip->pci_dev->dev,"\n" );
One of dev_dbg jobs is to print KERN_DEBUG, but only at the very beginning of the line.
drivers/char/tpm/tpm_infineon.c line 232
Quoted 8 lines
drivers/char/tpm/tpm_infineon.c +8
-0+dev_dbg (&chip->pci_dev->dev,"Receiving data: " ); + +for (i =0 ; i < size; i++) { +wait (chip,STAT_RDA ); + buf[i] =inb (chip->vendor->base +RDFIFO ); +dev_dbg (&chip->pci_dev->dev,"%02x " , buf[i]); + } +dev_dbg (&chip->pci_dev->dev,"\n" );
dev_dbg() in the middle.
drivers/char/tpm/tpm_infineon.c line 266
Quoted 1 line
drivers/char/tpm/tpm_infineon.c +1
-0+ "-> Negative acknowledgement - retransmit commando!\n");
command?
drivers/char/tpm/tpm_infineon.c line 307
Quoted 5 lines
drivers/char/tpm/tpm_infineon.c +4
-0+static int tpm_inf_send (struct tpm_chip *chip,u8 * buf,size_t count) +{ + count_high =0 ; + count_low =0 ;
What's the point?
Quoted 9 lines
drivers/char/tpm/tpm_infineon.c +8
-0+ + count_high = (count &0xffff0000 ); + count_low = (count &0x0000ffff ); +dev_dbg (&chip->pci_dev->dev, +"Sending data %02x %02x %02x %02x %02x %02x " ,TPM_VL_VER , +TPM_VL_CHANNEL_TPM , count_4, count_3, count_2, count_1); + +wait_and_send (chip,TPM_VL_VER );
drivers/char/tpm/tpm_infineon.c line 327
Quoted 10 lines
drivers/char/tpm/tpm_infineon.c +10
-0+wait_and_send (chip,TPM_VL_CHANNEL_TPM ); +wait_and_send (chip, count_4); +wait_and_send (chip, count_3); +wait_and_send (chip, count_2); +wait_and_send (chip, count_1); + +for (i =0 ; i < count; i++) { +wait_and_send (chip, buf[i]); +dev_dbg (&chip->pci_dev->dev,"%02x " , buf[i]); + }
dev_dbg in the middle.
drivers/char/tpm/tpm_infineon.c line 431
Quoted 6 lines
drivers/char/tpm/tpm_infineon.c +5
-0+static int __init tpm_inf_probe (struct pci_dev *pci_dev, +const struct pci_device_id *pci_id) +{ +dev_dbg (&pci_dev->dev, +"Device activate register status : %x \n" , status);
%x\n
Hi!
Quoted 17 lines
after some corrections here is a newer patch supporting the Infineon Trusted Platform Module SLD 9630 (TPM 1.1b), which is embedded on Intel-mainboards or in HP/Fujitsu-Siemens/Toshiba-Notebooks.
The module fits the interfaces created by IBM and was patched against the latest kernel-snapshot 2.6.13-rc2.
Further information about this module and a list of supported hardware can be found here: http://www.prosec.rub.de/tpmBest Regards,
Marcel SelhorstSigned-off-by Marcel Selhorst <[email protected]>diff -uprN
linux-2.6.12.2/drivers/char/tpm/tpm_infineon.clinux/drivers/char/tpm/tpm_infineon.c
drivers/char/tpm/tpm_infineon.c line 6
Quoted 6 lines
drivers/char/tpm/tpm_infineon.c +6
-0@@ -0,0 +1,505 @@ +/* +* Copyright (C) 2005 +* Marcel Selhorst <[email protected]> +* Applied Data Security Group +* Ruhr-University Bochum, Germany +*
Could you make this
drivers/char/tpm/tpm_infineon.c line 8
Quoted 2 lines
drivers/char/tpm/tpm_infineon.c +2
-0+* Copyright (C) 2005 Marcel Selhorst <[email protected]> +* Applied Data Security Group Ruhr-University Bochum, Germany
and make sure Description stands out a bit more (like maybe putting it first?)
Quoted 21 lines
drivers/char/tpm/tpm_infineon.c +20
-0+* Description: +* Device Driver for the Infineon Technologies +* SLD 9630 TT Trusted Platform Module +* Specifications at www.trustedcomputinggroup.org +* +* Project-Homepage: +* http://www.prosec.rub.de/tpm +* +* 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" + +/* Infineon specific delay definitions */ +enum infineon_tpm_delay { +TPM_MAX_WTX_PACKAGES =50 ,/* maximum number of WTX-packages */ +TPM_WTX_MSLEEP_TIME =20 ,/* msleep-Time for WTX-packages */
drivers/char/tpm/tpm_infineon.c line 37
Quoted 10 lines
drivers/char/tpm/tpm_infineon.c +9
-0+TPM_MSLEEP_TIME =3 ,/* msleep-Time --> Interval to check status register */ +TPM_MAX_TRIES =5000 /* gives number of max. msleep()-calls before + throwing timeout */ +}; +/* Infineon specific TPM data */ +enum infineon_tpm_specific { +TCPA_INFINEON_DEV_VEN_VALUE =0x15D1 , +TPM_DATA = (TPM_ADDR +1 ) &0xff +};
Ugh, is it just me or are you abusing enums a bit?
drivers/char/tpm/tpm_infineon.c line 381
Quoted 7 lines
drivers/char/tpm/tpm_infineon.c +7
-0+static int __init tpm_inf_probe (struct pci_dev *pci_dev, +const struct pci_device_id *pci_id) +{ + +int rc =0 ; +u8 ioh; +u8 iol;
Put these two on one line? Are you sure probe can't be called during runtime for some pci hotplug case?
Pavel
Signature · 1 line
teflon -- maybe it is a trademark, but it should not be.
Quoted 11 lines · Pavel Machek
Ugh, is it just me or are you abusing enums a bit?
drivers/char/tpm/tpm_infineon.c line 381
Quoted 7 lines
drivers/char/tpm/tpm_infineon.c +7
-0+static int __init tpm_inf_probe (struct pci_dev *pci_dev, +const struct pci_device_id *pci_id) +{ + +int rc =0 ; +u8 ioh; +u8 iol;Put these two on one line? Are you sure probe can't be called during runtime for some pci hotplug case?
__devinit should be used here, if for nothing else, then for sanity's sake.
Signature · 2 lines
Vojtech Pavlik SuSE Labs, SuSE CR