use continue self test

use continue self test

Change-Id: I4785a6326017c63d83a8eb153d6b90ee82e5f839

BUG=chromeos-partner:222
TEST=manual (build FW, make sure system boots and tpmc prints good values)

[Luigi Semenzato, 2010-09-09 00:16:50]

While this is surely fine for boot, I am not completely comfortable it will work
properly for S3 resume.  The reason is that the specs allows a command to fail
with a DOING_SELFTEST error if the command needs some TPM resource that has not
been tested yet.  Now, with NVRAM access and TPM_Extend, we know that they work
immediately after a ContinueSelfTest, and any other commands execute much later
when booting.  We also know that the Infineon full self test runs for a good
part of a second---so I don't know what will happen if user-level programs
access the TPM while still self-testing.

I guess we either go for it and see what happens, or we do a full self test at
S3 resume (and take the hit).  I can also investigate the code (both the driver
and trousers) to see if it copes with it.

On 2010/09/08 23:52:32, Randall Spangler wrote:
>

[Randall, 2010-09-09 00:26:07]

On Wed, Sep 8, 2010 at 5:16 PM, <semenzato@chromium.org> wrote:

> While this is surely fine for boot, I am not completely comfortable it will
> work
> properly for S3 resume.  The reason is that the specs allows a command to
> fail
> with a DOING_SELFTEST error if the command needs some TPM resource that has
> not
> been tested yet.  Now, with NVRAM access and TPM_Extend, we know that they
> work
> immediately after a ContinueSelfTest, and any other commands execute much
> later
> when booting.  We also know that the Infineon full self test runs for a
> good
> part of a second---so I don't know what will happen if user-level programs
> access the TPM while still self-testing.
>
> I guess we either go for it and see what happens, or we do a full self test
> at
> S3 resume (and take the hit).  I can also investigate the code (both the
> driver
> and trousers) to see if it copes with it.


SGTM for going for it.  If the code isn't tolerant of it yet, we should make
it tolerant.  Worst-case, won't your current TPM hack do a SelfTestFull at
driver-time?

- Randall


>
> On 2010/09/08 23:52:32, Randall Spangler wrote:
>
>
>
>
> http://codereview.chromium.org/3367020/show
>

[Luigi Semenzato, 2010-09-09 00:32:36]

Wait... I was being stupid.  We don't need to run the test after S3 because we
don't power the TPM off.   Let me check the current code.

On 2010/09/09 00:26:07, Randall wrote:
> On Wed, Sep 8, 2010 at 5:16 PM, <mailto:semenzato@chromium.org> wrote:
> 
> > While this is surely fine for boot, I am not completely comfortable it will
> > work
> > properly for S3 resume.  The reason is that the specs allows a command to
> > fail
> > with a DOING_SELFTEST error if the command needs some TPM resource that has
> > not
> > been tested yet.  Now, with NVRAM access and TPM_Extend, we know that they
> > work
> > immediately after a ContinueSelfTest, and any other commands execute much
> > later
> > when booting.  We also know that the Infineon full self test runs for a
> > good
> > part of a second---so I don't know what will happen if user-level programs
> > access the TPM while still self-testing.
> >
> > I guess we either go for it and see what happens, or we do a full self test
> > at
> > S3 resume (and take the hit).  I can also investigate the code (both the
> > driver
> > and trousers) to see if it copes with it.
> 
> 
> SGTM for going for it.  If the code isn't tolerant of it yet, we should make
> it tolerant.  Worst-case, won't your current TPM hack do a SelfTestFull at
> driver-time?
> 
> - Randall
> 
> 
> >
> > On 2010/09/08 23:52:32, Randall Spangler wrote:
> >
> >
> >
> >
> > http://codereview.chromium.org/3367020/show
> >
>

[Luigi Semenzato, 2010-09-09 00:40:34]

OK, here's my proposal.  Let's be conservative and always do the full self test
on S3 resume.  That code will eventually be dead anyway, it's only there to
support legacy prototype hardware.  Do we care if S3 resume will take an extra
0.7s on those machines?  Do we need to look further in the future? (i.e. the
possibility that future hardware will power off the TPM by design?)

On 2010/09/09 00:32:36, Luigi Semenzato wrote:
> Wait... I was being stupid.  We don't need to run the test after S3 because we
> don't power the TPM off.   Let me check the current code.
> 
> On 2010/09/09 00:26:07, Randall wrote:
> > On Wed, Sep 8, 2010 at 5:16 PM, <mailto:semenzato@chromium.org> wrote:
> > 
> > > While this is surely fine for boot, I am not completely comfortable it
will
> > > work
> > > properly for S3 resume.  The reason is that the specs allows a command to
> > > fail
> > > with a DOING_SELFTEST error if the command needs some TPM resource that
has
> > > not
> > > been tested yet.  Now, with NVRAM access and TPM_Extend, we know that they
> > > work
> > > immediately after a ContinueSelfTest, and any other commands execute much
> > > later
> > > when booting.  We also know that the Infineon full self test runs for a
> > > good
> > > part of a second---so I don't know what will happen if user-level programs
> > > access the TPM while still self-testing.
> > >
> > > I guess we either go for it and see what happens, or we do a full self
test
> > > at
> > > S3 resume (and take the hit).  I can also investigate the code (both the
> > > driver
> > > and trousers) to see if it copes with it.
> > 
> > 
> > SGTM for going for it.  If the code isn't tolerant of it yet, we should make
> > it tolerant.  Worst-case, won't your current TPM hack do a SelfTestFull at
> > driver-time?
> > 
> > - Randall
> > 
> > 
> > >
> > > On 2010/09/08 23:52:32, Randall Spangler wrote:
> > >
> > >
> > >
> > >
> > > http://codereview.chromium.org/3367020/show
> > >
> >

[gauravsh, 2010-09-09 00:47:31]

On Wed, Sep 8, 2010 at 5:40 PM,  <semenzato@chromium.org> wrote:
> OK, here's my proposal.  Let's be conservative and always do the full self
> test
> on S3 resume.  That code will eventually be dead anyway, it's only there to
> support legacy prototype hardware.  Do we care if S3 resume will take an
> extra
> 0.7s on those machines?  Do we need to look further in the future? (i.e. the
> possibility that future hardware will power off the TPM by design?)

To answer that question, Yes. 0.7 seconds is a long time to be adding
to the resume. Users are more likely to do an S3 resume than a full
reboot. So resume time matters and we probably want to aim for much
less than what we have right now.

>
> On 2010/09/09 00:32:36, Luigi Semenzato wrote:
>>
>> Wait... I was being stupid.  We don't need to run the test after S3
>> because we
>> don't power the TPM off.   Let me check the current code.
>
>> On 2010/09/09 00:26:07, Randall wrote:
>> > On Wed, Sep 8, 2010 at 5:16 PM, <mailto:semenzato@chromium.org> wrote:
>> >
>> > > While this is surely fine for boot, I am not completely comfortable it
>
> will
>>
>> > > work
>> > > properly for S3 resume.  The reason is that the specs allows a command
>> > > to
>> > > fail
>> > > with a DOING_SELFTEST error if the command needs some TPM resource
>> > > that
>
> has
>>
>> > > not
>> > > been tested yet.  Now, with NVRAM access and TPM_Extend, we know that
>> > > they
>> > > work
>> > > immediately after a ContinueSelfTest, and any other commands execute
>> > > much
>> > > later
>> > > when booting.  We also know that the Infineon full self test runs for
>> > > a
>> > > good
>> > > part of a second---so I don't know what will happen if user-level
>> > > programs
>> > > access the TPM while still self-testing.
>> > >
>> > > I guess we either go for it and see what happens, or we do a full self
>
> test
>>
>> > > at
>> > > S3 resume (and take the hit).  I can also investigate the code (both
>> > > the
>> > > driver
>> > > and trousers) to see if it copes with it.
>> >
>> >
>> > SGTM for going for it.  If the code isn't tolerant of it yet, we should
>> > make
>> > it tolerant.  Worst-case, won't your current TPM hack do a SelfTestFull
>> > at
>> > driver-time?
>> >
>> > - Randall
>> >
>> >
>> > >
>> > > On 2010/09/08 23:52:32, Randall Spangler wrote:
>> > >
>> > >
>> > >
>> > >
>> > > http://codereview.chromium.org/3367020/show
>> > >
>> >
>
>
>
> http://codereview.chromium.org/3367020/show
>



-- 
-g

[Luigi Semenzato, 2010-09-09 00:53:53]

Some more info: it appears that neither the driver or trousers deal with the
DOING_SELFTEST error (which they could do by waiting and retrying).  I doubt
that applications deal with it.  It would be reasonable to make this change to
the driver and upstream it.  In any case I propose that in our firmware we do a
SelfTestFull for now.

On 2010/09/09 00:40:34, Luigi Semenzato wrote:
> OK, here's my proposal.  Let's be conservative and always do the full self
test
> on S3 resume.  That code will eventually be dead anyway, it's only there to
> support legacy prototype hardware.  Do we care if S3 resume will take an extra
> 0.7s on those machines?  Do we need to look further in the future? (i.e. the
> possibility that future hardware will power off the TPM by design?)
> 
> On 2010/09/09 00:32:36, Luigi Semenzato wrote:
> > Wait... I was being stupid.  We don't need to run the test after S3 because
we
> > don't power the TPM off.   Let me check the current code.
> > 
> > On 2010/09/09 00:26:07, Randall wrote:
> > > On Wed, Sep 8, 2010 at 5:16 PM, <mailto:semenzato@chromium.org> wrote:
> > > 
> > > > While this is surely fine for boot, I am not completely comfortable it
> will
> > > > work
> > > > properly for S3 resume.  The reason is that the specs allows a command
to
> > > > fail
> > > > with a DOING_SELFTEST error if the command needs some TPM resource that
> has
> > > > not
> > > > been tested yet.  Now, with NVRAM access and TPM_Extend, we know that
they
> > > > work
> > > > immediately after a ContinueSelfTest, and any other commands execute
much
> > > > later
> > > > when booting.  We also know that the Infineon full self test runs for a
> > > > good
> > > > part of a second---so I don't know what will happen if user-level
programs
> > > > access the TPM while still self-testing.
> > > >
> > > > I guess we either go for it and see what happens, or we do a full self
> test
> > > > at
> > > > S3 resume (and take the hit).  I can also investigate the code (both the
> > > > driver
> > > > and trousers) to see if it copes with it.
> > > 
> > > 
> > > SGTM for going for it.  If the code isn't tolerant of it yet, we should
make
> > > it tolerant.  Worst-case, won't your current TPM hack do a SelfTestFull at
> > > driver-time?
> > > 
> > > - Randall
> > > 
> > > 
> > > >
> > > > On 2010/09/08 23:52:32, Randall Spangler wrote:
> > > >
> > > >
> > > >
> > > >
> > > > http://codereview.chromium.org/3367020/show
> > > >
> > >

[Luigi Semenzato, 2010-09-09 01:00:07]

To answer Gaurav's comment: I agree 0.7s is a long time for a production device,
but this would happen only on the current prototypes.  Is that acceptable?  If
in future production devices we decide to turn off the TPM during S3, we'll have
to revisit this choice, but it seems fair to postpone this.

On 2010/09/09 00:53:53, Luigi Semenzato wrote:
> Some more info: it appears that neither the driver or trousers deal with the
> DOING_SELFTEST error (which they could do by waiting and retrying).  I doubt
> that applications deal with it.  It would be reasonable to make this change to
> the driver and upstream it.  In any case I propose that in our firmware we do
a
> SelfTestFull for now.
> 
> On 2010/09/09 00:40:34, Luigi Semenzato wrote:
> > OK, here's my proposal.  Let's be conservative and always do the full self
> test
> > on S3 resume.  That code will eventually be dead anyway, it's only there to
> > support legacy prototype hardware.  Do we care if S3 resume will take an
extra
> > 0.7s on those machines?  Do we need to look further in the future? (i.e. the
> > possibility that future hardware will power off the TPM by design?)
> > 
> > On 2010/09/09 00:32:36, Luigi Semenzato wrote:
> > > Wait... I was being stupid.  We don't need to run the test after S3
because
> we
> > > don't power the TPM off.   Let me check the current code.
> > > 
> > > On 2010/09/09 00:26:07, Randall wrote:
> > > > On Wed, Sep 8, 2010 at 5:16 PM, <mailto:semenzato@chromium.org> wrote:
> > > > 
> > > > > While this is surely fine for boot, I am not completely comfortable it
> > will
> > > > > work
> > > > > properly for S3 resume.  The reason is that the specs allows a command
> to
> > > > > fail
> > > > > with a DOING_SELFTEST error if the command needs some TPM resource
that
> > has
> > > > > not
> > > > > been tested yet.  Now, with NVRAM access and TPM_Extend, we know that
> they
> > > > > work
> > > > > immediately after a ContinueSelfTest, and any other commands execute
> much
> > > > > later
> > > > > when booting.  We also know that the Infineon full self test runs for
a
> > > > > good
> > > > > part of a second---so I don't know what will happen if user-level
> programs
> > > > > access the TPM while still self-testing.
> > > > >
> > > > > I guess we either go for it and see what happens, or we do a full self
> > test
> > > > > at
> > > > > S3 resume (and take the hit).  I can also investigate the code (both
the
> > > > > driver
> > > > > and trousers) to see if it copes with it.
> > > > 
> > > > 
> > > > SGTM for going for it.  If the code isn't tolerant of it yet, we should
> make
> > > > it tolerant.  Worst-case, won't your current TPM hack do a SelfTestFull
at
> > > > driver-time?
> > > > 
> > > > - Randall
> > > > 
> > > > 
> > > > >
> > > > > On 2010/09/08 23:52:32, Randall Spangler wrote:
> > > > >
> > > > >
> > > > >
> > > > >
> > > > > http://codereview.chromium.org/3367020/show
> > > > >
> > > >

[Luigi Semenzato, 2010-09-09 16:48:52]

LGTM with either SelfTestFull or ContinueSelfTest on S3 resume.