* [PATCH] 2.6.26-mmotm tpm-correct-tpm-timeouts-to-jiffies-conversion-d820-fix.patch
[not found] ` <20080725185115.5d1a107d.akpm@linux-foundation.org>
@ 2008-07-26 4:03 ` Valdis.Kletnieks
2008-07-26 19:27 ` Marcin Obara
0 siblings, 1 reply; 4+ messages in thread
From: Valdis.Kletnieks @ 2008-07-26 4:03 UTC (permalink / raw)
To: Andrew Morton, Linus Torvalds, Marcin Obara
Cc: linux-kernel, Marcel Selhorst, Kylene Jo Hall, tpmdd-devel
[-- Attachment #1: Type: text/plain, Size: 1274 bytes --]
Patch tpm-correct-tpm-timeouts-to-jiffies-conversion reveals a bug in the
Broadcom BCM0102 TPM chipset used in the Dell Latitude D820 - although most of
the timeouts are returned in usecs as per the spec, one is apparently returned
in msecs, which results in a timeout when the code treats it as usecs. To
prevent a regression, we check for the known too-short value and adjust it to a
value that makes things work.
Signed-off-by: Valdis Kletnieks <valdis.kletnieks@vt.edu>
cc: Marcin Obara <marcin_obara@users.sourceforge.net>
--- linux-2.6.26-mmotm-0724/drivers/char/tpm/tpm.c.broadcom 2008-07-25 22:13:47.000000000 -0400
+++ linux-2.6.26-mmotm-0724/drivers/char/tpm/tpm.c 2008-07-25 23:30:38.000000000 -0400
@@ -557,6 +557,13 @@ duration:
usecs_to_jiffies(be32_to_cpu
(*((__be32 *) (data +
TPM_GET_CAP_RET_UINT32_1_IDX))));
+ /* The Broadcom BCM0102 chipset in a Dell Latitude D820 gets the above
+ * value wrong and apparently reports msecs rather than usecs. Test
+ * for the specific buggy value and adjust it to prevent a regression.
+ */
+ if (chip->vendor.duration[TPM_SHORT] == 1)
+ chip->vendor.duration[TPM_SHORT] *= 1000;
+
chip->vendor.duration[TPM_MEDIUM] =
usecs_to_jiffies(be32_to_cpu
(*((__be32 *) (data +
[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] 2.6.26-mmotm tpm-correct-tpm-timeouts-to-jiffies-conversion-d820-fix.patch
2008-07-26 4:03 ` [PATCH] 2.6.26-mmotm tpm-correct-tpm-timeouts-to-jiffies-conversion-d820-fix.patch Valdis.Kletnieks
@ 2008-07-26 19:27 ` Marcin Obara
2008-08-11 20:44 ` Andrew Morton
2008-08-15 9:39 ` Valdis.Kletnieks
0 siblings, 2 replies; 4+ messages in thread
From: Marcin Obara @ 2008-07-26 19:27 UTC (permalink / raw)
To: Valdis.Kletnieks
Cc: Andrew Morton, Linus Torvalds, linux-kernel, Marcel Selhorst,
Kylene Jo Hall, tpmdd-devel
2008/7/26 <Valdis.Kletnieks@vt.edu>:
> + if (chip->vendor.duration[TPM_SHORT] == 1)
> + chip->vendor.duration[TPM_SHORT] *= 1000;
> +
> chip->vendor.duration[TPM_MEDIUM] =
> usecs_to_jiffies(be32_to_cpu
> (*((__be32 *) (data +
Value in chip->vendor.duration[TPM_SHORT] is in jiffies not in milliseconds.
(As I know it's not the same. Jiffy is in range 1-10 ms.)
I know the result may be the same, but it is unclear.
Maybe... value should be compared (to 1000) before conversion?
or...
If after conversion, there should be something like this:
if (chip->vendor.duration[TPM_SHORT] < (HZ/100)) /* less
than 10ms ? */
chip->vendor.duration[TPM_SHORT] = HZ;
What do you think?
Regards
Marcin
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] 2.6.26-mmotm tpm-correct-tpm-timeouts-to-jiffies-conversion-d820-fix.patch
2008-07-26 19:27 ` Marcin Obara
@ 2008-08-11 20:44 ` Andrew Morton
2008-08-15 9:39 ` Valdis.Kletnieks
1 sibling, 0 replies; 4+ messages in thread
From: Andrew Morton @ 2008-08-11 20:44 UTC (permalink / raw)
To: Marcin Obara
Cc: Valdis.Kletnieks, torvalds, linux-kernel, tpm, kjhall, tpmdd-devel
On Sat, 26 Jul 2008 20:27:33 +0100
"Marcin Obara" <marcin_obara@users.sourceforge.net> wrote:
> 2008/7/26 <Valdis.Kletnieks@vt.edu>:
> > + if (chip->vendor.duration[TPM_SHORT] == 1)
> > + chip->vendor.duration[TPM_SHORT] *= 1000;
> > +
> > chip->vendor.duration[TPM_MEDIUM] =
> > usecs_to_jiffies(be32_to_cpu
> > (*((__be32 *) (data +
>
>
> Value in chip->vendor.duration[TPM_SHORT] is in jiffies not in milliseconds.
> (As I know it's not the same. Jiffy is in range 1-10 ms.)
> I know the result may be the same, but it is unclear.
>
> Maybe... value should be compared (to 1000) before conversion?
> or...
> If after conversion, there should be something like this:
> if (chip->vendor.duration[TPM_SHORT] < (HZ/100)) /* less
> than 10ms ? */
> chip->vendor.duration[TPM_SHORT] = HZ;
>
> What do you think?
>
This all seemed to die off without a conclusion.
Here's the current
tpm-correct-tpm-timeouts-to-jiffies-conversion.patch. Does it need
updating?
Thanks.
From: Marcin Obara <marcin_obara@users.sourceforge.net>
This patch fixes timeouts conversion to jiffies, by replacing
msecs_to_jiffies() calls with usecs_to_jiffies(). According to TCG TPM
Specification Version 1.2 Revision 103 (pages 166, 167) TPM timeouts and
durations are returned in microseconds (usec) not in miliseconds (msec).
Signed-off-by: Marcin Obara <marcin_obara@users.sourceforge.net>
Cc: Marcel Selhorst <tpm@selhorst.net>
Cc: Kylene Jo Hall <kjhall@us.ibm.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
drivers/char/tpm/tpm.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff -puN drivers/char/tpm/tpm.c~tpm-correct-tpm-timeouts-to-jiffies-conversion drivers/char/tpm/tpm.c
--- a/drivers/char/tpm/tpm.c~tpm-correct-tpm-timeouts-to-jiffies-conversion
+++ a/drivers/char/tpm/tpm.c
@@ -525,19 +525,19 @@ void tpm_get_timeouts(struct tpm_chip *c
timeout =
be32_to_cpu(*((__be32 *) (data + TPM_GET_CAP_RET_UINT32_1_IDX)));
if (timeout)
- chip->vendor.timeout_a = msecs_to_jiffies(timeout);
+ chip->vendor.timeout_a = usecs_to_jiffies(timeout);
timeout =
be32_to_cpu(*((__be32 *) (data + TPM_GET_CAP_RET_UINT32_2_IDX)));
if (timeout)
- chip->vendor.timeout_b = msecs_to_jiffies(timeout);
+ chip->vendor.timeout_b = usecs_to_jiffies(timeout);
timeout =
be32_to_cpu(*((__be32 *) (data + TPM_GET_CAP_RET_UINT32_3_IDX)));
if (timeout)
- chip->vendor.timeout_c = msecs_to_jiffies(timeout);
+ chip->vendor.timeout_c = usecs_to_jiffies(timeout);
timeout =
be32_to_cpu(*((__be32 *) (data + TPM_GET_CAP_RET_UINT32_4_IDX)));
if (timeout)
- chip->vendor.timeout_d = msecs_to_jiffies(timeout);
+ chip->vendor.timeout_d = usecs_to_jiffies(timeout);
duration:
memcpy(data, tpm_cap, sizeof(tpm_cap));
@@ -554,15 +554,15 @@ duration:
return;
chip->vendor.duration[TPM_SHORT] =
- msecs_to_jiffies(be32_to_cpu
+ usecs_to_jiffies(be32_to_cpu
(*((__be32 *) (data +
TPM_GET_CAP_RET_UINT32_1_IDX))));
chip->vendor.duration[TPM_MEDIUM] =
- msecs_to_jiffies(be32_to_cpu
+ usecs_to_jiffies(be32_to_cpu
(*((__be32 *) (data +
TPM_GET_CAP_RET_UINT32_2_IDX))));
chip->vendor.duration[TPM_LONG] =
- msecs_to_jiffies(be32_to_cpu
+ usecs_to_jiffies(be32_to_cpu
(*((__be32 *) (data +
TPM_GET_CAP_RET_UINT32_3_IDX))));
}
_
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] 2.6.26-mmotm tpm-correct-tpm-timeouts-to-jiffies-conversion-d820-fix.patch
2008-07-26 19:27 ` Marcin Obara
2008-08-11 20:44 ` Andrew Morton
@ 2008-08-15 9:39 ` Valdis.Kletnieks
1 sibling, 0 replies; 4+ messages in thread
From: Valdis.Kletnieks @ 2008-08-15 9:39 UTC (permalink / raw)
To: Marcin Obara
Cc: Andrew Morton, Linus Torvalds, linux-kernel, Marcel Selhorst,
Kylene Jo Hall, tpmdd-devel
[-- Attachment #1: Type: text/plain, Size: 1076 bytes --]
On Sat, 26 Jul 2008 20:27:33 BST, Marcin Obara said:
> Value in chip->vendor.duration[TPM_SHORT] is in jiffies not in milliseconds.
> (As I know it's not the same. Jiffy is in range 1-10 ms.)
> I know the result may be the same, but it is unclear.
I suppose I could have worded the comment block better - the intent was to
point out what the Broadcom chip returns, but by that point in the code
we're dealing with jiffies...
> Maybe... value should be compared (to 1000) before conversion?
Actually, that's probably a better idea, because my kernel is built with
HZ=1000 - usecs_to_jiffies will do something different than ==1 for HZ=100
or HZ=250 or other odd values.
> If after conversion, there should be something like this:
> if (chip->vendor.duration[TPM_SHORT] < (HZ/100)) /* less
> than 10ms ? */
> chip->vendor.duration[TPM_SHORT] = HZ;
That's another option as well, that does the right thing for various HZ values.
> What do you think?
Let me go cook up and test another iteration of the patch, will probably be
a few hours...
[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2008-08-15 13:27 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20080725164614.044a526e.akpm@linux-foundation.org>
[not found] ` <50684.1217035060@turing-police.cc.vt.edu>
[not found] ` <20080725185115.5d1a107d.akpm@linux-foundation.org>
2008-07-26 4:03 ` [PATCH] 2.6.26-mmotm tpm-correct-tpm-timeouts-to-jiffies-conversion-d820-fix.patch Valdis.Kletnieks
2008-07-26 19:27 ` Marcin Obara
2008-08-11 20:44 ` Andrew Morton
2008-08-15 9:39 ` Valdis.Kletnieks
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).