tpm + tpm_tis : Various fixes
cover tpm + tpm_tis : Various fixes
The following is a series of patches that fix various issues in the general TPM driver and in the TPM TIS driver. I had posted some of the patches previously, but I am now reposting all of them again as part of this series. Some previously posted patches have seen further changes (also to conform to the coding style).
All patches apply to the current tip of the main git tree.
Some of the changes are related to using timeouts that the TPM reports, others are related to fixing the interrupt mode in which the TPM TIS driver can be run.
Another patch introduces the automatic probing for the Intel iTPM flaw. The probing is useful in combination with the 'force' module parameter that enables the interrupt mode on some machines but then ends up circumventing ACPI to determine whether an Intel iTPM is present to activate the work-around.
I have tested these patches on four different types of machines. They worked fine there even though the TPM could not be used in interrupt mode on all of them (IRQ line not connected?).
The patches apply in the order they appear.
Regards,
Stefan
PATCH 1/8 tpm_tis: Use timeouts returned from TPM
v3:
- sysfs entry now called 'durations' to resemble TPM-speak (previously was called 'timeouts')
v2:
- adjusting all timeouts for TPM devices reporting timeouts in msec rather than usec
- also displaying in sysfs whether the timeouts are 'original' or 'adjusted'
The current TPM TIS driver in git discards the timeout values returned from the TPM. The check of the response packet needs to consider that the return_code field is 0 on success and the size of the expected packet is equivalent to the header size + u32 length indicator for the TPM_GetCapability() result + 3 timeout indicators of type u32.
Since some TPMs seem to return timeouts in msec rather than usec, I am now adjusting all the timeouts rather than just the one for short durations.
I am also adding a sysfs entry 'durations' showing the timeouts that are being used.
--- drivers/char/tpm/tpm.c | 38 ++++++++++++++++++++++++++++++-------- drivers/char/tpm/tpm.h | 3 +++ drivers/char/tpm/tpm_tis.c | 4 +++- 3 files changed, 36 insertions(+), 9 deletions(-)
drivers/char/tpm/tpm.c +30 -8
@@ -575,23 +575,31 @@ duration:if (rc)return ; -if (be32_to_cpu (tpm_cmd.header.out.return_code) - !=3 *sizeof (u32)) +if (be32_to_cpu (tpm_cmd.header.out.return_code) !=0 || +be32_to_cpu (tpm_cmd.header.out.length) + !=sizeof (tpm_cmd.header.out) +sizeof (u32) +3 *sizeof (u32))return ; + duration_cap = &tpm_cmd.params.getcap_out.cap.duration; chip->vendor.duration[TPM_SHORT ] =usecs_to_jiffies (be32_to_cpu (duration_cap->tpm_short)); + chip->vendor.duration[TPM_MEDIUM ] = +usecs_to_jiffies (be32_to_cpu (duration_cap->tpm_medium)); + chip->vendor.duration[TPM_LONG ] = +usecs_to_jiffies (be32_to_cpu (duration_cap->tpm_long)); +/* The Broadcom BCM0102 chipset in a Dell Latitude D820 gets the above * value wrong and apparently reports msecs rather than usecs. So we * fix up the resulting too-small TPM_SHORT value to make things work. +* We also scale the TPM_MEDIUM and -_LONG values by 1000. */ -if (chip->vendor.duration[TPM_SHORT ] < (HZ /100 )) +if (chip->vendor.duration[TPM_SHORT ] < (HZ /100 )) { chip->vendor.duration[TPM_SHORT ] =HZ ; - - chip->vendor.duration[TPM_MEDIUM ] = -usecs_to_jiffies (be32_to_cpu (duration_cap->tpm_medium)); - chip->vendor.duration[TPM_LONG ] = -usecs_to_jiffies (be32_to_cpu (duration_cap->tpm_long)); + chip->vendor.duration[TPM_MEDIUM ] *=1000 ; + chip->vendor.duration[TPM_LONG ] *=1000 ; + chip->vendor.duration_adjusted = true; +dev_info (chip->dev,"Adjusting TPM timeout parameters." ); + } }EXPORT_SYMBOL_GPL (tpm_get_timeouts);
@@ -937,6 +945,20 @@ ssize_t tpm_show_caps_1_2(struct device }EXPORT_SYMBOL_GPL (tpm_show_caps_1_2); +ssize_t tpm_show_durations (struct device *dev,struct device_attribute *attr, +char *buf) +{ +struct tpm_chip *chip =dev_get_drvdata (dev); + +return sprintf (buf,"%d %d %d [%s]\n" , +jiffies_to_usecs (chip->vendor.duration[TPM_SHORT ]), +jiffies_to_usecs (chip->vendor.duration[TPM_MEDIUM ]), +jiffies_to_usecs (chip->vendor.duration[TPM_LONG ]), + chip->vendor.duration_adjusted + ?"adjusted" :"original" ); +} +EXPORT_SYMBOL_GPL (tpm_show_durations); +ssize_t tpm_store_cancel (struct device *dev,struct device_attribute *attr,const char *buf,size_t count) {
drivers/char/tpm/tpm.h +3 -0
@@ -56,6 +56,8 @@ extern ssize_t tpm_show_owned(struct dev char *);extern ssize_t tpm_show_temp_deactivated (struct device *,struct device_attribute *attr,char *); +extern ssize_t tpm_show_durations (struct device *, +struct device_attribute *attr,char *);struct tpm_chip ;
@@ -82,6 +84,7 @@ struct tpm_vendor_specific {int locality;unsigned long timeout_a, timeout_b, timeout_c, timeout_d;/* jiffies */ unsigned long duration[3 ];/* jiffies */ +bool duration_adjusted;wait_queue_head_t read_queue;wait_queue_head_t int_queue;
drivers/char/tpm/tpm_tis.c +3 -1
@@ -376,6 +376,7 @@ static DEVICE_ATTR(temp_deactivated, S_I NULL); static DEVICE_ATTR(caps, S_IRUGO, tpm_show_caps_1_2, NULL); static DEVICE_ATTR(cancel, S_IWUSR | S_IWGRP, NULL, tpm_store_cancel); +static DEVICE_ATTR(durations, S_IRUGO, tpm_show_durations, NULL);static struct attribute *tis_attrs[] = { &dev_attr_pubek.attr,
@@ -385,7 +386,8 @@ static struct attribute *tis_attrs[] = { &dev_attr_owned.attr, &dev_attr_temp_deactivated.attr, &dev_attr_caps.attr, - &dev_attr_cancel.attr, NULL, + &dev_attr_cancel.attr, + &dev_attr_durations.attr, NULL, }; static struct attribute_group tis_attr_grp = {
Re: [patch 1/8] tpm_tis: Use timeouts returned from TPM
Hi Stefan,
Some comments:
Quoted 7 lines · Stefan Berger
v3:
- sysfs entry now called 'durations' to resemble TPM-speak (previously was called 'timeouts')v2:
- adjusting all timeouts for TPM devices reporting timeouts in msec rather than usec
This is a bugfix that's different than the one you originally sent, can you submit this as a separated patch? This is mainly for easier debug in the future, so each feature/bugfix/commit can be tested separately.
Quoted 2 lines
- also displaying in sysfs whether the timeouts are 'original' or 'adjusted'
Minor comment, it's easier to read the log when the changelog comes after the main patch description.
Quoted 130 lines
The current TPM TIS driver in git discards the timeout values returned from the TPM. The check of the response packet needs to consider that the
return_codefield is 0 on success and the size of the expected packet is equivalent to the header size + u32 length indicator for theTPM_GetCapability()result + 3 timeout indicators of type u32.Since some TPMs seem to return timeouts in msec rather than usec, I am now adjusting all the timeouts rather than just the one for short durations.
I am also adding a sysfs entry 'durations' showing the timeouts that are being used.
Signed-off-by Stefan Berger <[email protected]>--- drivers/char/tpm/tpm.c | 38 ++++++++++++++++++++++++++++++-------- drivers/char/tpm/tpm.h | 3 +++ drivers/char/tpm/tpm_tis.c | 4 +++- 3 files changed, 36 insertions(+), 9 deletions(-)Index:
linux-2.6/drivers/char/tpm/tpm.c
===================================================================drivers/char/tpm/tpm.c +30
-8@@ -575,23 +575,31 @@ duration:if (rc)return ; -if (be32_to_cpu (tpm_cmd.header.out.return_code) - !=3 *sizeof (u32)) +if (be32_to_cpu (tpm_cmd.header.out.return_code) !=0 || +be32_to_cpu (tpm_cmd.header.out.length) + !=sizeof (tpm_cmd.header.out) +sizeof (u32) +3 *sizeof (u32))return ; + duration_cap = &tpm_cmd.params.getcap_out.cap.duration; chip->vendor.duration[TPM_SHORT ] =usecs_to_jiffies (be32_to_cpu (duration_cap->tpm_short)); + chip->vendor.duration[TPM_MEDIUM ] = +usecs_to_jiffies (be32_to_cpu (duration_cap->tpm_medium)); + chip->vendor.duration[TPM_LONG ] = +usecs_to_jiffies (be32_to_cpu (duration_cap->tpm_long)); +/* The Broadcom BCM0102 chipset in a Dell Latitude D820 gets the above * value wrong and apparently reports msecs rather than usecs. So we * fix up the resulting too-small TPM_SHORT value to make things work. +* We also scale the TPM_MEDIUM and -_LONG values by 1000. */ -if (chip->vendor.duration[TPM_SHORT ] < (HZ /100 )) +if (chip->vendor.duration[TPM_SHORT ] < (HZ /100 )) { chip->vendor.duration[TPM_SHORT ] =HZ ; - - chip->vendor.duration[TPM_MEDIUM ] = -usecs_to_jiffies (be32_to_cpu (duration_cap->tpm_medium)); - chip->vendor.duration[TPM_LONG ] = -usecs_to_jiffies (be32_to_cpu (duration_cap->tpm_long)); + chip->vendor.duration[TPM_MEDIUM ] *=1000 ; + chip->vendor.duration[TPM_LONG ] *=1000 ; + chip->vendor.duration_adjusted = true; +dev_info (chip->dev,"Adjusting TPM timeout parameters." ); + } }EXPORT_SYMBOL_GPL (tpm_get_timeouts);@@ -937,6 +945,20 @@ ssize_t tpm_show_caps_1_2(struct device }EXPORT_SYMBOL_GPL (tpm_show_caps_1_2); +ssize_t tpm_show_durations (struct device *dev,struct device_attribute *attr, +char *buf) +{ +struct tpm_chip *chip =dev_get_drvdata (dev); + +return sprintf (buf,"%d %d %d [%s]\n" , +jiffies_to_usecs (chip->vendor.duration[TPM_SHORT ]), +jiffies_to_usecs (chip->vendor.duration[TPM_MEDIUM ]), +jiffies_to_usecs (chip->vendor.duration[TPM_LONG ]), + chip->vendor.duration_adjusted + ?"adjusted" :"original" ); +} +EXPORT_SYMBOL_GPL (tpm_show_durations); +ssize_t tpm_store_cancel (struct device *dev,struct device_attribute *attr,const char *buf,size_t count) {Index:
linux-2.6/drivers/char/tpm/tpm.h
===================================================================drivers/char/tpm/tpm.h +3
-0@@ -56,6 +56,8 @@ extern ssize_t tpm_show_owned(struct dev char *);extern ssize_t tpm_show_temp_deactivated (struct device *,struct device_attribute *attr,char *); +extern ssize_t tpm_show_durations (struct device *, +struct device_attribute *attr,char *);struct tpm_chip ;@@ -82,6 +84,7 @@ struct tpm_vendor_specific {int locality;unsigned long timeout_a, timeout_b, timeout_c, timeout_d;/* jiffies */ unsigned long duration[3 ];/* jiffies */ +bool duration_adjusted;wait_queue_head_t read_queue;wait_queue_head_t int_queue;Index:
linux-2.6/drivers/char/tpm/tpm_tis.c
===================================================================drivers/char/tpm/tpm_tis.c +3
-1@@ -376,6 +376,7 @@ static DEVICE_ATTR(temp_deactivated, S_I NULL); static DEVICE_ATTR(caps, S_IRUGO, tpm_show_caps_1_2, NULL); static DEVICE_ATTR(cancel, S_IWUSR | S_IWGRP, NULL, tpm_store_cancel); +static DEVICE_ATTR(durations, S_IRUGO, tpm_show_durations, NULL);static struct attribute *tis_attrs[] = { &dev_attr_pubek.attr,@@ -385,7 +386,8 @@ static struct attribute *tis_attrs[] = { &dev_attr_owned.attr, &dev_attr_temp_deactivated.attr, &dev_attr_caps.attr, - &dev_attr_cancel.attr, NULL, + &dev_attr_cancel.attr, + &dev_attr_durations.attr, NULL, }; static struct attribute_group tis_attr_grp = {
Re: [patch 1/8] tpm_tis: Use timeouts returned from TPM
Quoted 9 lines · Rajiv Andrade
Hi Stefan,
Some comments:
Quoted 4 lines · Stefan Berger
v3:
- sysfs entry now called 'durations' to resemble TPM-speak (previously was called 'timeouts')
Quoted 6 lines
Quoted 3 lines
v2:
- adjusting all timeouts for TPM devices reporting timeouts in msec rather than usecThis is a bugfix that's different than the one you originally sent, can you submit this as a separated patch? This is mainly for easier debug in the future, so each feature/bugfix/commit can be tested separately.
Originally I sent a patch that corrected the evaluation of the return code within the TPM's response and introduced the sysfs entry. I did not scale the MEDIUM and LONG timeouts, which then caused the problems on the Infineon TPM and the patch was removed. I then introduced the scaling of the MEDIUM and LONG timeouts, which seems necessary as a consequence to working with the TPM-reported timeouts. Then adding a sysfs entry to it is an additional feature. I'd split this patch in 2 patches where the 2nd one introduces the sysfs entry. Is this ok with you?
Stefan
PATCH 2/8 tpm_tis: Re-enable interrupts upon (S3) resume
In reply to Stefan Bergerv2:
- the patch was adapted to also work with a machine with a Intel iTPM
This patch makes sure that if the TPM TIS interface is run in interrupt mode (rather than polling mode) that all interrupts are enabled in the TPM's interrupt enable register after a resume from ACPI S3 suspend. The registers may either have been cleared by the TPM loosing its state during device sleep or by the BIOS leaving the TPM in polling mode (after sending a command to the TPM for starting it up again)
You may want to check if your TPM runs with interrupts by doing
cat /proc/interrupts | grep -i tpm
and see whether there is an entry or otherwise for it to use interrupts:
modprobe tpm_tis interrupts=1 [add 'itpm=1' for Intel TPM ]
--- drivers/char/tpm/tpm_tis.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+)
drivers/char/tpm/tpm_tis.c +30 -0
@@ -647,11 +647,36 @@ static int tpm_tis_pnp_suspend(struct pnreturn tpm_pm_suspend (&dev->dev, msg); } +static void tpm_tis_reenable_interrupts (struct tpm_chip *chip) +{ +u32 intmask; + +/* reenable interrupts that device may have lost or +BIOS/firmware may have disabled */ +iowrite8 (chip->vendor.irq, chip->vendor.iobase + +TPM_INT_VECTOR (chip->vendor.locality)); + + intmask = +ioread32 (chip->vendor.iobase + +TPM_INT_ENABLE (chip->vendor.locality)); + + intmask |=TPM_INTF_CMD_READY_INT + |TPM_INTF_LOCALITY_CHANGE_INT |TPM_INTF_DATA_AVAIL_INT + |TPM_INTF_STS_VALID_INT |TPM_GLOBAL_INT_ENABLE ; + +iowrite32 (intmask, + chip->vendor.iobase +TPM_INT_ENABLE (chip->vendor.locality)); +} + +static int tpm_tis_pnp_resume (struct pnp_dev *dev) {struct tpm_chip *chip =pnp_get_drvdata (dev);int ret; +if (chip->vendor.irq) +tpm_tis_reenable_interrupts (chip); + ret =tpm_pm_resume (&dev->dev);if (!ret)tpm_continue_selftest (chip);
@@ -704,6 +729,11 @@ static int tpm_tis_suspend(struct platfostatic int tpm_tis_resume (struct platform_device *dev) { +struct tpm_chip *chip =dev_get_drvdata (&dev->dev); + +if (chip->vendor.irq) +tpm_tis_reenable_interrupts (chip); +return tpm_pm_resume (&dev->dev); }static struct platform_driver tis_drv = {
Re: [patch 2/8] tpm_tis: Re-enable interrupts upon (S3) resume
Quoted 3 lines · Stefan Berger
v2:
- the patch was adapted to also work with a machine with a Intel iTPM
This is also a separated fix, which this time I can't also identify in the patch, can you submit it as another patch, that applies on top of the original 2/8?
Rajiv
Re: [patch 2/8] tpm_tis: Re-enable interrupts upon (S3) resume
Quoted 7 lines · Rajiv Andrade
Quoted 3 lines · Stefan Berger
v2:
- the patch was adapted to also work with a machine with a Intel iTPMThis is also a separated fix, which this time I can't also identify in the patch, can you submit it as another patch, that applies on top of the original 2/8?
There are two types of drivers in the tpm_tis driver, the pnp one and the 'platform' driver. The initial patch did not provide support for both of them, so maybe the comment above is misleading. I'll fix the comment, but don't think it makes sense tearing the patch into two pieces.
Stefan
Quoted 1 line
Rajiv
PATCH 3/8 tpm: Fix display of data in pubek sysfs entry
In reply to Stefan BergerThis patch fixes the TPM's pubek sysfs entry that is accessible as long as the TPM doesn't have an owner. I shifted the access to the data by -10 -- the first byte immediately follows the 10 byte header. The line
data = tpm_cmd.params.readpubek_out_buffer;
sets it at the offset '10' in the packet, so we can read the data array starting at offset '0'.
Before:
Algorithm: 00 0C 00 00
Encscheme: 08 00
Sigscheme: 00 00
Parameters: 00 00 00 00 01 00 AC E2 5E 3C A0 78
Modulus length: -563306801
Modulus:
28 21 08 0F 82 CD F2 B1 E7 49 F7 74 70 BE 59 8C
43 78 B1 24 EA 52 E2 FE 52 5C 3A 12 3B DC 61 71
[...]
After:
Algorithm: 00 00 00 01
Encscheme: 00 03
Sigscheme: 00 01
Parameters: 00 00 08 00 00 00 00 02 00 00 00 00
Modulus length: 256
Modulus:
AC E2 5E 3C A0 78 DE 6C 9E CF 28 21 08 0F 82 CD
F2 B1 E7 49 F7 74 70 BE 59 8C 43 78 B1 24 EA 52
[...]
--- drivers/char/tpm/tpm.c | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-)
drivers/char/tpm/tpm.c +16 -10
@@ -871,18 +871,24 @@ ssize_t tpm_show_pubek(struct device *de data = tpm_cmd.params.readpubek_out_buffer; str +=sprintf (str, -"Algorithm: %02X %02X %02X %02X\nEncscheme: %02X %02X\n" -"Sigscheme: %02X %02X\nParameters: %02X %02X %02X %02X" -" %02X %02X %02X %02X %02X %02X %02X %02X\n" -"Modulus length: %d\nModulus: \n" , - data[10 ], data[11 ], data[12 ], data[13 ], data[14 ], - data[15 ], data[16 ], data[17 ], data[22 ], data[23 ], - data[24 ], data[25 ], data[26 ], data[27 ], data[28 ], - data[29 ], data[30 ], data[31 ], data[32 ], data[33 ], -be32_to_cpu (*((__be32 *) (data +34 )))); +"Algorithm: %02X %02X %02X %02X\n" +"Encscheme: %02X %02X\n" +"Sigscheme: %02X %02X\n" +"Parameters: %02X %02X %02X %02X " +"%02X %02X %02X %02X " +"%02X %02X %02X %02X\n" +"Modulus length: %d\n" +"Modulus:\n" , + data[0 ], data[1 ], data[2 ], data[3 ], + data[4 ], data[5 ], + data[6 ], data[7 ], + data[12 ], data[13 ], data[14 ], data[15 ], + data[16 ], data[17 ], data[18 ], data[19 ], + data[20 ], data[21 ], data[22 ], data[23 ], +be32_to_cpu (*((__be32 *) (data +24 ))));for (i =0 ; i <256 ; i++) { - str +=sprintf (str,"%02X " , data[i +38 ]); + str +=sprintf (str,"%02X " , data[i +28 ]);if ((i +1 ) %16 ==0 ) str +=sprintf (str,"\n" ); }
PATCH 4/8 tpm_tis: Delay ACPI S3 suspend while TPM is busy
In reply to Stefan BergerThis patch delays the (ACPI S3) suspend while the TPM is busy processing a command and the TPM TIS driver is run in interrupt mode. This is the same behavior as we already have it for the TPM TIS driver in polling mode.
Reasoning: Some of the TPM's commands advance the internal state of the TPM. An example would be the extending of one of its PCR registers. Upper layers, such as IMA or TSS (TrouSerS), would certainly want to be sure that the command succeeded rather than getting an error code (-62 = -ETIME) that may not give a conclusive answer as for what reason the command failed. Reissuing such a command would put the TPM into the wrong state, so waiting for it to finish is really the only option.
The downside is that some commands (key creation) can take a long time and actually prevent the machine from entering S3 at all before the 20 second timeout of the power management subsystem arrives.
--- drivers/char/tpm/tpm_tis.c | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-)
drivers/char/tpm/tpm_tis.c +23 -5
@@ -26,6 +26,7 @@#include <linux/interrupt.h> #include <linux/wait.h> #include <linux/acpi.h> +#include <linux/freezer.h> #include "tpm.h" #define TPM_HEADER_SIZE 10
@@ -120,7 +121,7 @@ static void release_locality(struct tpm_static int request_locality (struct tpm_chip *chip,int l) { -unsigned long stop; +unsigned long stop, timeout;long rc;if (check_locality (chip, l) >=0 )
@@ -129,17 +130,25 @@ static int request_locality(struct tpm_ciowrite8 (TPM_ACCESS_REQUEST_USE , chip->vendor.iobase +TPM_ACCESS (l)); + stop = jiffies + chip->vendor.timeout_a; +if (chip->vendor.irq) { +again: + timeout = stop - jiffies; +if ((long )timeout <=0 ) +return -1 ; rc =wait_event_interruptible_timeout (chip->vendor.int_queue, (check_locality (chip, l) >=0 ), - chip->vendor.timeout_a); + timeout);if (rc >0 )return l; - +if (rc == -ERESTARTSYS &&freezing (current)) { +clear_thread_flag (TIF_SIGPENDING ); + goto again; + } }else {/* wait for burstcount */ - stop = jiffies + chip->vendor.timeout_a; do { if (check_locality(chip, l) >= 0) return l;
@@ -196,15 +205,24 @@ static int wait_for_stat(struct tpm_chipif ((status & mask) == mask)return 0 ; + stop = jiffies + timeout; +if (chip->vendor.irq) { +again: + timeout = stop - jiffies; +if ((long )timeout <=0 ) +return -ETIME ; rc =wait_event_interruptible_timeout (*queue, ((tpm_tis_status (chip) & mask) == mask), timeout);if (rc >0 )return 0 ; +if (rc == -ERESTARTSYS &&freezing (current)) { +clear_thread_flag (TIF_SIGPENDING ); + goto again; + } }else { - stop = jiffies + timeout; do { msleep(TPM_TIMEOUT); status = tpm_tis_status(chip);
PATCH 5/8 tpm_tis: Fix the probing for interrupts
In reply to Stefan BergerThis patch fixes several aspects of the probing for interrupts.
I am reading the TPM's timeouts before probing for the interrupts. The tpm_get_timeouts() function is invoked in polling mode and gets the proper timeouts from the TPM so that we don't need to fall back to 2 minutes timeouts for short duration commands while the interrupt probing is happening.
I am introducing a variable probed_irq into the vendor structure that gets the irq number if an interrupt is received while the the tpm_gen_interrupt() function is run in polling mode during interrupt probing. Previously some parts of tpm_gen_interrupt() were run in polling mode, then the irq variable was set in the interrupt handler when an interrupt was received and execution of tpm_gen_interrupt() ended up switching over to interrupt mode. tpm_gen_interrupt() execution ended up on an event queue where it eventually timed out since the probing handler doesn't wake any queues.
Before calling into free_irq() I am clearing all interrupt flags that may have been set by the TPM. The reason is that free_irq() will call into the probing interrupt handler and may otherwise fool us into thinking that a real interrupt happened (because we see the flags as being set) while the TPM's interrupt line is not even connected to anything on the motherboard. This solves a problem one one machine I did testing.
Further, if a TPM claims to use a specifc interrupt, also make sure that that interrupt is working by running the probing on it as well. If a TPM indicates that it does not use a specific interrupt (returns '0'), probe all interrupts from 3 to 15.
--- drivers/char/tpm/tpm.h | 1 + drivers/char/tpm/tpm_tis.c | 33 +++++++++++++++++++++++++++------ 2 files changed, 28 insertions(+), 6 deletions(-)
drivers/char/tpm/tpm_tis.c +27 -6
@@ -436,7 +436,7 @@ static irqreturn_t tis_int_probe(int irqif (interrupt ==0 )return IRQ_NONE ; - chip->vendor.irq = irq; + chip->vendor.probed_irq = irq;/* Clear interrupts handled with TPM_EOI */ iowrite32(interrupt,
@@ -484,7 +484,7 @@ static int tpm_tis_init(struct device *d resource_size_t len, unsigned int irq) {u32 vendor, intfcaps, intmask; -int rc, i; +int rc, i, irq_s, irq_e;struct tpm_chip *chip;if (!(chip =tpm_register_hardware (dev, &tpm_tis)))
@@ -542,6 +542,9 @@ static int tpm_tis_init(struct device *dif (intfcaps &TPM_INTF_DATA_AVAIL_INT )dev_dbg (dev,"\tData Avail Int Support\n" ); +/* get the timeouts before testing for irqs */ +tpm_get_timeouts (chip); +/* INTERRUPT Setup */ init_waitqueue_head (&chip->vendor.read_queue);init_waitqueue_head (&chip->vendor.int_queue);
@@ -560,13 +563,19 @@ static int tpm_tis_init(struct device *dif (interrupts) chip->vendor.irq = irq;if (interrupts && !chip->vendor.irq) { - chip->vendor.irq = + irq_s =ioread8 (chip->vendor.iobase +TPM_INT_VECTOR (chip->vendor.locality)); +if (irq_s) { + irq_e = irq_s; + }else { + irq_s =3 ; + irq_e =15 ; + } -for (i =3 ; i <16 && chip->vendor.irq ==0 ; i++) { +for (i = irq_s; i <= irq_e && chip->vendor.irq ==0 ; i++) {iowrite8 (i, chip->vendor.iobase + -TPM_INT_VECTOR (chip->vendor.locality)); +TPM_INT_VECTOR (chip->vendor.locality));if (request_irq (i, tis_int_probe,IRQF_SHARED , chip->vendor.miscdev.name, chip) !=0 ) {
@@ -588,9 +597,22 @@ static int tpm_tis_init(struct device *d chip->vendor.iobase + TPM_INT_ENABLE(chip->vendor.locality)); + chip->vendor.probed_irq =0 ; +/* Generate Interrupts */ tpm_gen_interrupt (chip); + chip->vendor.irq = chip->vendor.probed_irq; + +/* free_irq will call into tis_int_probe; +clear all irqs we haven't seen while doing +tpm_gen_interrupt */ +iowrite32 (ioread32 + (chip->vendor.iobase + +TPM_INT_STATUS (chip->vendor.locality)), + chip->vendor.iobase + +TPM_INT_STATUS (chip->vendor.locality)); +/* Turn off */ iowrite32(intmask, chip->vendor.iobase +
@@ -629,7 +651,6 @@ static int tpm_tis_init(struct device *dlist_add (&chip->vendor.list, &tis_chips);spin_unlock (&tis_lock); -tpm_get_timeouts (chip);tpm_continue_selftest (chip);return 0 ;
drivers/char/tpm/tpm.h +1 -0
@@ -69,6 +69,7 @@ struct tpm_vendor_specific {unsigned long base;/* TPM base address */ int irq; +int probed_irq;int region_size;int have_region;
PATCH 6/8 tpm + tpm_tis: Use interface timeouts returned from TPM
In reply to Stefan BergerThis patch fixes the check for whether the 4 interface timeouts (a,b,c,d) reported by the TPM have been properly returned.
The current TPM TIS driver in git discards the interface timeout values returned from the TPM. The check of the response packet needs to consider that the return_code field is 0 on success and the size of the expected packet is equivalent to the header size + u32 length indicator for the TPM_GetCapability() result + 4 timeout indicators of type u32.
I anticipate that some TPMs will also report these timeouts in msec rather than usec and therefore scale them by a factor of 1000 if the timeout_a value is found to be below 1000 (usecs).
I am also add a sysfs entry 'timeouts' that shows the 4 timeouts and reports them either as being either 'original' or 'adjusted' if they were scaled.
--- drivers/char/tpm/tpm.c | 36 +++++++++++++++++++++++++++++------- drivers/char/tpm/tpm.h | 3 +++ drivers/char/tpm/tpm_tis.c | 4 +++- 3 files changed, 35 insertions(+), 8 deletions(-)
drivers/char/tpm/tpm.c +29 -7
@@ -534,6 +534,7 @@ void tpm_get_timeouts(struct tpm_chip *cstruct duration_t *duration_cap;ssize_t rc;u32 timeout; +unsigned int scale =1 ; tpm_cmd.header.in = tpm_getcap_header; tpm_cmd.params.getcap_in.cap =TPM_CAP_PROP ;
@@ -545,24 +546,30 @@ void tpm_get_timeouts(struct tpm_chip *cif (rc) goto duration; -if (be32_to_cpu (tpm_cmd.header.out.length) - !=4 *sizeof (u32)) - goto duration; +if (be32_to_cpu (tpm_cmd.header.out.return_code) !=0 || +be32_to_cpu (tpm_cmd.header.out.length) + !=sizeof (tpm_cmd.header.out) +sizeof (u32) +4 *sizeof (u32)) +return ; timeout_cap = &tpm_cmd.params.getcap_out.cap.timeout;/* Don't overwrite default if value is 0 */ timeout =be32_to_cpu (timeout_cap->a); +if (timeout && timeout <1000 ) { +/* timeouts in msec rather usec */ + scale =1000 ; + chip->vendor.timeout_adjusted = true; + }if (timeout) - chip->vendor.timeout_a =usecs_to_jiffies (timeout); + chip->vendor.timeout_a =usecs_to_jiffies (timeout * scale); timeout =be32_to_cpu (timeout_cap->b);if (timeout) - chip->vendor.timeout_b =usecs_to_jiffies (timeout); + chip->vendor.timeout_b =usecs_to_jiffies (timeout * scale); timeout =be32_to_cpu (timeout_cap->c);if (timeout) - chip->vendor.timeout_c =usecs_to_jiffies (timeout); + chip->vendor.timeout_c =usecs_to_jiffies (timeout * scale); timeout =be32_to_cpu (timeout_cap->d);if (timeout) - chip->vendor.timeout_d =usecs_to_jiffies (timeout); + chip->vendor.timeout_d =usecs_to_jiffies (timeout * scale); duration: tpm_cmd.header.in = tpm_getcap_header;
@@ -965,6 +972,21 @@ ssize_t tpm_show_durations(struct device }EXPORT_SYMBOL_GPL (tpm_show_durations); +ssize_t tpm_show_timeouts (struct device *dev,struct device_attribute *attr, +char *buf) +{ +struct tpm_chip *chip =dev_get_drvdata (dev); + +return sprintf (buf,"%d %d %d %d [%s]\n" , +jiffies_to_usecs (chip->vendor.timeout_a), +jiffies_to_usecs (chip->vendor.timeout_b), +jiffies_to_usecs (chip->vendor.timeout_c), +jiffies_to_usecs (chip->vendor.timeout_d), + chip->vendor.timeout_adjusted + ?"adjusted" :"original" ); +} +EXPORT_SYMBOL_GPL (tpm_show_timeouts); +ssize_t tpm_store_cancel (struct device *dev,struct device_attribute *attr,const char *buf,size_t count) {
drivers/char/tpm/tpm.h +3 -0
@@ -58,6 +58,8 @@ extern ssize_t tpm_show_temp_deactivated struct device_attribute *attr, char *);extern ssize_t tpm_show_durations (struct device *,struct device_attribute *attr,char *); +extern ssize_t tpm_show_timeouts (struct device *, +struct device_attribute *attr,char *);struct tpm_chip ;
@@ -84,6 +86,7 @@ struct tpm_vendor_specific {struct list_head list;int locality;unsigned long timeout_a, timeout_b, timeout_c, timeout_d;/* jiffies */ +bool timeout_adjusted;unsigned long duration[3 ];/* jiffies */ bool duration_adjusted;
drivers/char/tpm/tpm_tis.c +3 -1
@@ -395,6 +395,7 @@ static DEVICE_ATTR(temp_deactivated, S_I static DEVICE_ATTR(caps, S_IRUGO, tpm_show_caps_1_2, NULL); static DEVICE_ATTR(cancel, S_IWUSR | S_IWGRP, NULL, tpm_store_cancel); static DEVICE_ATTR(durations, S_IRUGO, tpm_show_durations, NULL); +static DEVICE_ATTR(timeouts, S_IRUGO, tpm_show_timeouts, NULL);static struct attribute *tis_attrs[] = { &dev_attr_pubek.attr,
@@ -405,7 +406,8 @@ static struct attribute *tis_attrs[] = { &dev_attr_temp_deactivated.attr, &dev_attr_caps.attr, &dev_attr_cancel.attr, - &dev_attr_durations.attr, NULL, + &dev_attr_durations.attr, + &dev_attr_timeouts.attr, NULL, }; static struct attribute_group tis_attr_grp = {
PATCH 7/8 tpm_tis: Probing function for Intel iTPM bug
In reply to Stefan BergerThis patch introduces a function for automatic probing for the Intel iTPM STS_DATA_EXPECT flaw.
I split the current tpm_tis_send function into 2 parts where the 1st part is now called tpm_tis_send_data() and merely sends the data to the TPM. I am using that function for probing. The new tpm_tis_send function now first calls tpm_tis_send_data and if that succeeds has the TPM process the command and waits until the response is there.
The probing for the Intel iTPM is only invoked if the user has not passed itpm=1 as parameter for the module *or* if such a TPM was detected via ACPI. Previously it was necessary to pass itpm=1 when also passing force=1 to the module when doing a 'modprobe'. This function is more general than the ACPI test function and the function relying on ACPI could probably be removed.
--- drivers/char/tpm/tpm_tis.c | 77 ++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 73 insertions(+), 4 deletions(-)
drivers/char/tpm/tpm_tis.c +73 -4
@@ -306,11 +306,10 @@ MODULE_PARM_DESC(itpm, "Force iTPM worka* tpm.c can skip polling for the data to be available as the interrupt is * waited for here */ -static int tpm_tis_send (struct tpm_chip *chip,u8 *buf,size_t len) +static int tpm_tis_send_data (struct tpm_chip *chip,u8 *buf,size_t len) {int rc, status, burstcnt;size_t count =0 ; -u32 ordinal;if (request_locality (chip,0 ) <0 )return -EBUSY ;
@@ -345,8 +344,7 @@ static int tpm_tis_send(struct tpm_chip/* write last byte */ iowrite8 (buf[count], - chip->vendor.iobase + -TPM_DATA_FIFO (chip->vendor.locality)); + chip->vendor.iobase +TPM_DATA_FIFO (chip->vendor.locality));wait_for_stat (chip,TPM_STS_VALID , chip->vendor.timeout_c, &chip->vendor.int_queue); status =tpm_tis_status (chip);
@@ -355,6 +353,28 @@ static int tpm_tis_send(struct tpm_chip goto out_err; } +return 0 ; + +out_err: +tpm_tis_ready (chip); +release_locality (chip, chip->vendor.locality,0 ); +return rc; +} + +/* +* If interrupts are used (signaled by an irq set in the vendor structure) +* tpm.c can skip polling for the data to be available as the interrupt is +* waited for here +*/ +static int tpm_tis_send (struct tpm_chip *chip,u8 *buf,size_t len) +{ +int rc; +u32 ordinal; + + rc =tpm_tis_send_data (chip, buf, len); +if (rc <0 ) +return rc; +/* go and do it */ iowrite8 (TPM_STS_GO , chip->vendor.iobase +TPM_STS (chip->vendor.locality));
@@ -376,6 +396,47 @@ out_err:return rc; } +/* +* Early probing for iTPM with STS_DATA_EXPECT flaw. +* Try sending command without itpm flag set and if that +* fails, repeat with itpm flag set. +*/ +static int probe_itpm (struct tpm_chip *chip) +{ +int rc =0 ; +u8 cmd_getticks[] = { +0x00 ,0xc1 ,0x00 ,0x00 ,0x00 ,0x0a , +0x00 ,0x00 ,0x00 ,0xf1 + }; +size_t len =sizeof (cmd_getticks); +int rem_itpm = itpm; + + itpm =0 ; + + rc =tpm_tis_send_data (chip, cmd_getticks, len); +if (rc ==0 ) + goto out; + +tpm_tis_ready (chip); +release_locality (chip, chip->vendor.locality,0 ); + + itpm =1 ; + + rc =tpm_tis_send_data (chip, cmd_getticks, len); +if (rc ==0 ) { +dev_info (chip->dev,"Detected an iTPM.\n" ); + rc =1 ; + }else + rc = -EFAULT ; + +out: + itpm = rem_itpm; +tpm_tis_ready (chip); +release_locality (chip, chip->vendor.locality,0 ); + +return rc; +} +static const struct file_operations tis_ops = { .owner =THIS_MODULE , .llseek = no_llseek,
@@ -515,6 +576,14 @@ static int tpm_tis_init(struct device *d"1.2 TPM (device-id 0x%X, rev-id %d)\n" , vendor >> 16, ioread8(chip->vendor.iobase + TPM_RID(0))); +if (!itpm) { + itpm =probe_itpm (chip); +if (itpm <0 ) { + rc = -ENODEV ; + goto out_err; + } + } +if (itpm)dev_info (dev,"Intel iTPM workaround enabled\n" );
PATCH 8/8 tpm: Fix a typo
In reply to Stefan BergerThis patch fixes a typo.
drivers/char/tpm/tpm.c +1 -1
@@ -615,7 +615,7 @@ void tpm_continue_selftest(struct tpm_chu8 data[] = {0 ,193 ,/* TPM_TAG_RQU_COMMAND */ 0 ,0 ,0 ,10 ,/* length */ -0 ,0 ,0 ,83 ,/* TPM_ORD_GetCapability */ +0 ,0 ,0 ,83 ,/* TPM_ORD_ContinueSelfTest */ };tpm_transmit (chip, data,sizeof (data));