Move ContinueSelfTest to a later point to save time.

Move ContinueSelfTest to a later point to save time.

Change-Id: I96b413438359e11315101d408033066e6f0a0981

BUG=chrome-os-partner: 1826
TEST=none

Committed: http://chrome-svn/viewvc/chromeos?view=rev&revision=06fbb16

[Luigi Semenzato, 2011-03-16 18:57:46]

I have a lunch meeting, but you can start looking at this.
I don't know where to put the TPM-specific #defines.
Thanks!

[Randall Spangler, 2011-03-16 19:23:06]

http://codereview.chromium.org/6667051/diff/1003/firmware/lib/rollback_index.c
File firmware/lib/rollback_index.c (right):

http://codereview.chromium.org/6667051/diff/1003/firmware/lib/rollback_index....
firmware/lib/rollback_index.c:188: * For TPMs in the intersection of these two
sets, we're screwed.
"we're screwed" --> "TPMs that require manually starting the self-test AND block
will have poor performance until we split TlclSendReceive() into Send() and
Receive(), and have a state machine to control setup."

http://codereview.chromium.org/6667051/diff/1003/firmware/lib/tpm_lite/tlcl.c
File firmware/lib/tpm_lite/tlcl.c (right):

http://codereview.chromium.org/6667051/diff/1003/firmware/lib/tpm_lite/tlcl.c...
firmware/lib/tpm_lite/tlcl.c:105: } while (result == TPM_E_DOING_SELFTEST);
If the TPM gets in a wonky state, this could block forever and brick the system.
 I'd rather have the command fail and throw us into recovery mode than lock up.

One way to handle that would be to pass an allow_retries flag down through
everything (because sigh, no global variables), and then turn that off if the
recovery button is pressed.

[Luigi Semenzato, 2011-03-16 20:46:04]

PTAL when you get a chance.


On 2011/03/16 19:23:06, Randall Spangler wrote:
> http://codereview.chromium.org/6667051/diff/1003/firmware/lib/rollback_index.c
> File firmware/lib/rollback_index.c (right):
> 
>
http://codereview.chromium.org/6667051/diff/1003/firmware/lib/rollback_index....
> firmware/lib/rollback_index.c:188: * For TPMs in the intersection of these two
> sets, we're screwed.
> "we're screwed" --> "TPMs that require manually starting the self-test AND
block
> will have poor performance until we split TlclSendReceive() into Send() and
> Receive(), and have a state machine to control setup."
> 
> http://codereview.chromium.org/6667051/diff/1003/firmware/lib/tpm_lite/tlcl.c
> File firmware/lib/tpm_lite/tlcl.c (right):
> 
>
http://codereview.chromium.org/6667051/diff/1003/firmware/lib/tpm_lite/tlcl.c...
> firmware/lib/tpm_lite/tlcl.c:105: } while (result == TPM_E_DOING_SELFTEST);
> If the TPM gets in a wonky state, this could block forever and brick the
system.
>  I'd rather have the command fail and throw us into recovery mode than lock
up.
> 
> One way to handle that would be to pass an allow_retries flag down through
> everything (because sigh, no global variables), and then turn that off if the
> recovery button is pressed.

[Luigi Semenzato, 2011-03-16 20:51:58]

Forgot to publish...

http://codereview.chromium.org/6667051/diff/1003/firmware/lib/rollback_index.c
File firmware/lib/rollback_index.c (right):

http://codereview.chromium.org/6667051/diff/1003/firmware/lib/rollback_index....
firmware/lib/rollback_index.c:188: * For TPMs in the intersection of these two
sets, we're screwed.
On 2011/03/16 19:23:06, Randall Spangler wrote:
> "we're screwed" --> "TPMs that require manually starting the self-test AND
block
> will have poor performance until we split TlclSendReceive() into Send() and
> Receive(), and have a state machine to control setup."

OK, but I thought mine was more concise and to the point. :-)

http://codereview.chromium.org/6667051/diff/1003/firmware/lib/tpm_lite/tlcl.c
File firmware/lib/tpm_lite/tlcl.c (right):

http://codereview.chromium.org/6667051/diff/1003/firmware/lib/tpm_lite/tlcl.c...
firmware/lib/tpm_lite/tlcl.c:105: } while (result == TPM_E_DOING_SELFTEST);
On 2011/03/16 19:23:06, Randall Spangler wrote:
> If the TPM gets in a wonky state, this could block forever and brick the
system.
>  I'd rather have the command fail and throw us into recovery mode than lock
up.
> 
> One way to handle that would be to pass an allow_retries flag down through
> everything (because sigh, no global variables), and then turn that off if the
> recovery button is pressed.

Ah, now I remember, that's why we wanted the compilation-time flag.

[Randall Spangler, 2011-03-16 21:44:54]

LGTM, assuming we set TPM_BLOCKING_CONTINUESELFTEST in the BIOS.

[Luigi Semenzato, 2011-03-16 23:08:48]

I think we're done with this.  PTAL.

[Randall Spangler, 2011-03-16 23:23:43]

LGTM assuming suitably reworded comment.

http://codereview.chromium.org/6667051/diff/2003/firmware/lib/tpm_lite/tlcl.c
File firmware/lib/tpm_lite/tlcl.c (right):

http://codereview.chromium.org/6667051/diff/2003/firmware/lib/tpm_lite/tlcl.c...
firmware/lib/tpm_lite/tlcl.c:91: /* When compiling for the firmware, hide
command failures due to the self
???

When compiling for the firmware, we look at the failures and attempt retry.

When compiling for the host, we pass through the failures without retry.

Neither case hides the failure.

[Luigi Semenzato, 2011-03-17 00:33:54]

Fixed comment and pushed.  Thanks.

On Wed, Mar 16, 2011 at 4:23 PM,  <rspangler@chromium.org> wrote:
> LGTM assuming suitably reworded comment.
>
>
>
> http://codereview.chromium.org/6667051/diff/2003/firmware/lib/tpm_lite/tlcl.c
> File firmware/lib/tpm_lite/tlcl.c (right):
>
>
http://codereview.chromium.org/6667051/diff/2003/firmware/lib/tpm_lite/tlcl.c...
> firmware/lib/tpm_lite/tlcl.c:91: /* When compiling for the firmware,
> hide command failures due to the self
> ???
>
> When compiling for the firmware, we look at the failures and attempt
> retry.
>
> When compiling for the host, we pass through the failures without retry.
>
> Neither case hides the failure.
>
> http://codereview.chromium.org/6667051/
>