[U-Boot] [PATCH] TPM: remove dead code

[Simon Glass]

Hi Wolfgang,

On Wed, Nov 7, 2012 at 6:08 AM, Wolfgang Denk <wd at denx.de> wrote:
> Dear Simon Glass,
>
> In message <CAPnjgZ2QyRjfjbJTR9MHcsQUEV933oAWPjSTvv-kFrhLCFJRUg at mail.gmail.com> you wrote:
>>
>> OK, but I still don't quite get it. As I asked in the other thread,
>> are you not interested in TPM functionality at all until we have a
>> verified boot implementation, or are you happy to have things progress
>> in stages? I'm will to work through this, and I have the time at
>> present, but I believe that drivers for two common TPM chips are a
>> useful addition to U-Boot regardless.
>
> I'm sorry, but I disagree - the drivers may be useful indeed, but as
> long as there are no users in U-Boot they are just dead weight.
>
> So far we have followed the rule not to add dead code - we always asked
> to add code (drivers) together with any "consumers", i. e. other code
> that provides useful features (at least to one board) that actually
> needs and uses such drivers.
>
> With TPM we made an exception from this rule, based on the expectation
> that users would be added "soon".  Then a full year passed, and
> nothing happened.  Really _nothing_.
>
> Now you suggest to "progress in stages"?  You mean adding more and
> more code around, all of which will be unused, until one bright day in
> a non-foreseeable future some board might get added, that will then
> use this?

Please see my comment below.

>
> This is not exactly a proposal that triggers enthusiasm to me.
>

Enthusiasm is a strong word...

>
>> It's not obvious how to mainline this rather large feature except in
>> pieces. For example, I think I can start on a feature to verify a
>> kernel, but I do want to talk to the TPM as part of that.
>
> Instead of working bottom up (which will necessarily result in an
> approach trying to add unused code) you could do it top down: start
> indeed with a feature to verify a kernel (oh! we already have that
> as psrt of the image verification - with simple CRC in the leggacy
> images, and will optionally all other kinds in FIT images).  This
> "verification could initially be an empty function just returning a
> "true" value.  No TPM is needed for that.  Than work top down for
> there.

OK I will see if I can do something there. The TPM is used to verify
the kernel version to prevent rollback attacks.

>
>> That's going to be a big series.... but in contrast, display, SATA and
>> GPIO patches are ok, but TPM is not?
>
> Ummm... are you telling me that contrast, display, SATA and GPIO
> patches all also add dead code that is not used anywhere?  I have not
> looked closer there because I simply did not expoect this.  But if so,
> the very same rules apply:  we do not want to add dead code.

Let's take an example. We have AHCI/SATA support in U-Boot. There may
be no board code in U-Boot that actually reads from a disk (I am not
sure, but it is plausible, since hard-coded disk/fs access from a
board's C code would be odd). Let's say this code exists only in
commands that may or may not be executed by board scripts, which are
out of tree. I'm sure this isn't considered dead code, but I want to
understand the difference. If there is a TPM command which may or may
not be executed by board scripts (also out of tree), what is the
difference?

Following this dead code argument, whole U-Boot subsystems are dead
code since they are only used indirectly from the command line and not
directly by any board code?

So my thinking now is that I should:

1. Create a way of extracting verification information from a signed
block (e.g. kernel hash, rollback info)
2. Add some commands to access this
3. Create a basic script which uses these commands

Does this sound right?

>
>> I'll see what I can do here, but in principle I feel that adding a
>> driver should be OK, provided a board uses it, without necessarily the
>> high-level code that uses it. After all, we don't necessarily expect
>> to mainline all the scripts that drive the actual boot.
>
> what exactly do you mean by "a board uses it, without necessarily the
> high-level code that uses it"?  If "uses" for you means just
> conteining a function call reference so the code gets cimpiled and
> pulled in by the linker, then I have to tell you that this NOT my
> understanding of "usage".
>
> For me, "using" some code means that it actually performs some
> _function_.  As long as the code is not actually running on the
> system, it is not being used.

OK, see above, I think we are getting to the root of this.

>
>> > And especially not after the experience with the TPM code so far,
>> > where my patience has been exhausted after more than a year of nothing
>> > happening.
>>
>> Well not nothing. Have been rather tied up getting a product out. I
>
> Sorry, but for U-Boot it is completely irrelevant what happens with
> your out of tree code.  The code in mainline has just been sitting
> there, unused as ever.
>
>> agree that things have been very quiet on verified boot - perhaps the
>> original upstream author was exhausted after the effort of getting the
>> driver into mainline and retired for a rest :-)
>
> Well, this experience has sunk in.  Exactly what I feared has
> happened: the code was added, and it has been dead ever since,
> for more than a full year.
>
> It will be pretty difficult to talk me again into accepting unused
> code.
>

So long as I can understand the definition then we should be fine.

>
>> However most of the common/ series is code that we depend on and I
>> think might be generally useful. I do admit that in striving for a
>> very high level of polish we have attacked problems that perhaps would
>> not matter for many systems, and thus more patches.
>
> Mentioning that you depend on some specific code does not actually
> make me feel bad, if this is what you (subcounsciously) had in mind.
> Quote David Woodhouse: "If you're out of tree, you don't exist."
> ( http://article.gmane.org/gmane.linux.kernel.embedded/3534 )
>

Well if it is not generally useful then we shouldn't have it. But it's
good to post things to the list and have people comment.

>
> All this discussion confirmed my resolution not to allow for dead
> code.
>
> Best regards,
>
> Wolfgang Denk
>

Regards,
Simon

> --
> DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
> "The question of whether a computer can think is no more  interesting
> than the question of whether a submarine can swim"
>                                                 - Edsgar W.  Dijkstra