Re: [REGRESSION] tpm_tis on Lenovo T410 broken in 2.6.38-rc6

[Rajiv Andrade <srajiv@linux.vnet.ibm.com>]

Hi Ted, sorry for the delay.

On 03/01/2011 06:37 PM, Ted Ts'o wrote:
> On Tue, Mar 01, 2011 at 06:14:25PM -0300, Rajiv Andrade wrote:
>> I was referring to the first, 9b29050f, and the commitc4ff4b82  reverted by Ted fixes a bug for other users.
> What's the bug that other users suffer by having a longer timeout?  It
> means that if things fails it takes longer for them to get an error
> message in the case of something that's going to fail anyway, right?
> Somehow that seems less serious than the TPM simply completely failing
> to function caused by too-short timeouts.  (My situation).  So sorry
> if I'm feeling selfish, but a longer timeout before returning failure
> seems less important than my not being able to access the TPM at all.
The bug was that when running the kernel with IMA, at boot time, it 
issues 3 TPM commands IIRC, given the 2 min timeout,
when the TPM didn't respond due to it not working with interrupts for 
example, the boot hang for 6 minutes. The TPM device
driver then relies on the default values, given neither 
tpm_get_timeouts() can succeed. Unfortunately, yes, I don't have a
fix for TPMs not working with interrupts yet, however we've been making 
progress there recently, should be out soon.

I understand, and I didn't mean one was more or less important than the 
other, just gave the reason I wrote it at that time,
considering that I wasn't aware of the effect you're facing.
You're not being selfish from my POV also, both bugs need to be fixed.

> So, the results of my experiments.  First of all, trying to merge in
> your for_james branch doesn't help at all.  I used the tip of Linus's
> tree as of this morning (commit 3e1f2356ce2) and then merged in your
> for-james tree (commit b83452256e).  No go.  I tried setting the
> interrupts parameter to both 0 and 1, and either way, things still
> didn't work.
>
> OK, next, I tried Linus's idea, which was to put a WARN macro here in
> tpm_calc_ordinal_duration:
>
>          if (duration_idx != TPM_UNDEFINED) {
> 		duration = chip->vendor.duration[duration_idx];
>                  /* if duration is 0, it's because chip->vendor.duration wasn't \
> */
>                  /* filled yet, so we set the lowest timeout just to give enough\
>   */
>                  /* time for tpm_get_timeouts() to succeed */
> NEW ---->	WARN(duration<=0,
> 		    "no duration for ordinal %x (diration_idx %d)",
> 		    ordinal, duration_idx);
>                  return (duration<= 0 ? HZ : duration);
> 	} else
>                  return 2 * 60 * HZ;
> }
>
> That resulted in the following data (see attached), which you will
> hopefully find useful.
Thanks, it is. HZ isn't enough time for this TPM/setup to have short 
timeout commands to succeed, including
the tpm_get_timeouts(). I was skeptic at first that this would be the 
reason since I have the same machine,
and was working for me, the reason I asked for these parameters setup 
attempts. I'll write a patch that will
contemplate the long boot time timeout and this bugs fixed, pursuing the 
interrupts fix path.

Rajiv