lkml.org 
[lkml]   [2018]   [Mar]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/5] tpm: Trigger only missing TPM 2.0 self tests
On Sat, Mar 03, 2018 at 12:11:59PM -0800, James Bottomley wrote:
> I still think removing all logging traces is a mistake for something
> that can consume a significant amount of time in the boot sequence.
>  It's going to cause lost of people doing boot timings to waste lots of
> effort.
>
> However, removing the log messages makes the above statement a lie, so
> one of the two needs fixing.

The commit itself makes sense but the implementation was sloppy to say
the least in v3. It was like prototype/PoC version of something that
could be merged to mainline, not something that can be merged to
mainline.

For example:

* You could have implemented it more cleanly without that new
'tpm_transmit_check' helper function as can be seen.
* Many log messages contained a redundant "TPM:" prefix.
* There duplicate logs to tpm_transmit_cmd(). Pass a NULL as
desc tpm_transmit_cmd() if you want to take care of logging
yourself.
* The commit has the same short summary as the commit it fixes.
This issue still persists. Open for suggestions.

Please state if v3 contains a log message that has been removed and is
still mandatory and I can add it if it makes sense.

> > + rc = tpm_transmit_cmd(chip, NULL, buf.data,
> > PAGE_SIZE, 0, 0,
> > +       "attempting the self test\n");
>
> There shouldn't be a \n in the string: the failure message already
> appends one.

Oops, my bad, will fix it.

/Jarkko

\
 
 \ /
  Last update: 2018-03-05 12:24    [W:0.052 / U:1.284 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site