Skip to contentSkip to conversation

TPM: remove dead code

PATCH New Wolfgang Denk 3 participants 17 messages Nov 3, 2012 – Nov 28, 2012

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 TPM: remove dead code

The TPM code was added more than a year or 4 releases ago. This was done under the proposition that board support that would actually use such code would be added soon. However, nothing happened since. The code has no users in mainline, and does not even get build for any configuration, so we cannot even tell if it compiles at all.

Remove the unused code. In in some far future actual users show up, it can be re-added easily.

Cc Vadim Bendebury <[email protected]>
Cc Simon Glass <[email protected]>
---
 README                        |  10 -
 common/Makefile               |   1 -
 common/cmd_tpm.c              | 103 ---------
 doc/driver-model/UDM-tpm.txt  |  48 ----
 drivers/tpm/Makefile          |  43 ----
 drivers/tpm/generic_lpc_tpm.c | 495 ------------------------------------------
 include/tpm.h                 |  71 ------
 7 files changed, 771 deletions(-)
 delete mode 100644 common/cmd_tpm.c
 delete mode 100644 doc/driver-model/UDM-tpm.txt
 delete mode 100644 drivers/tpm/Makefile
 delete mode 100644 drivers/tpm/generic_lpc_tpm.c
 delete mode 100644 include/tpm.h
README +0 -10
index 22fd6b7..54a0bdd 100644
@@ -1161,16 +1161,6 @@ The following options need to be configured:
 			CONFIG_SH_ETHER_CACHE_WRITEBACK
 			If this option is set, the driver enables cache flush.
 
-- TPM Support:
-		CONFIG_GENERIC_LPC_TPM
-		Support for generic parallel port TPM devices. Only one device
-		per system is supported at this time.
-
-			CONFIG_TPM_TIS_BASE_ADDRESS
-			Base address where the generic TPM device is mapped
-			to. Contemporary x86 systems usually map it at
-			0xfed40000.
-
 - USB Support:
 		At the moment only the UHCI host controller is
 		supported (PIP405, MIP405, MPC5200); define
common/Makefile +0 -1
index 9e43322..6bcfdca 100644
@@ -154,7 +154,6 @@ COBJS-$(CONFIG_CMD_STRINGS) += cmd_strings.o
 COBJS-$(CONFIG_CMD_TERMINAL) += cmd_terminal.o
 COBJS-$(CONFIG_CMD_TIME) += cmd_time.o
 COBJS-$(CONFIG_SYS_HUSH_PARSER) += cmd_test.o
-COBJS-$(CONFIG_CMD_TPM) += cmd_tpm.o
 COBJS-$(CONFIG_CMD_TSI148) += cmd_tsi148.o
 COBJS-$(CONFIG_CMD_UBI) += cmd_ubi.o
 COBJS-$(CONFIG_CMD_UBIFS) += cmd_ubifs.o
common/cmd_tpm.c deleted +0 -103

103 lines not on this page · raw message

doc/driver-model/UDM-tpm.txt deleted +0 -48

48 lines not on this page · raw message

drivers/tpm/Makefile deleted +0 -43

43 lines not on this page · raw message

drivers/tpm/generic_lpc_tpm.c deleted +0 -495

495 lines not on this page · raw message

include/tpm.h deleted +0 -71

71 lines not on this page · raw message

Signature · 1 line
1.7.11.7
#1 lore raw

PATCH 1/1 TPM: remove dead code

Hi Wolfgang,

Quoted 8 lines · Wolfgang Denk

The TPM code was added more than a year or 4 releases ago. This was done under the proposition that board support that would actually use such code would be added soon. However, nothing happened since. The code has no users in mainline, and does not even get build for any configuration, so we cannot even tell if it compiles at all.

Remove the unused code. In in some far future actual users show up, it can be re-added easily.

I think you may have missed the pending patches which make use of this. it is important functionality for the Chromebooks (secure boot).

Regards,
Simon

Quoted 5 lines
Cc Vadim Bendebury <[email protected]>
Cc Simon Glass <[email protected]>

---

#2 lore raw

PATCH 1/1 TPM: remove dead code

Dear Simon,

Quoted 8 lines · In message <CAPnjgZ0JLarq6r=sHe+cfbjkyuf6hRgGP4BR9u_aSug14MZcrg@mail.gmail.com>…
Quoted 6 lines · Wolfgang Denk

The TPM code was added more than a year or 4 releases ago. This was done under the proposition that board support that would actually use such code would be added soon. However, nothing happened since. The code has no users in mainline, and does not even get build for any configuration, so we cannot even tell if it compiles at all.

Quoted 5 lines
Quoted 2 lines

Remove the unused code. In in some far future actual users show up, it can be re-added easily.

I think you may have missed the pending patches which make use of this. it is important functionality for the Chromebooks (secure boot).

No, I have not missed these. But all the patch does is set CONFIG_GENERIC_LPC_TPM - there is still not a single user defining CONFIG_CMD_TPM, so what does this help? We still have tons of dead code around. Dump it!

Best regards,

Wolfgang Denk

Signature · 5 lines
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
Unix: Some say the learning curve is steep,  but  you  only  have  to
climb it once.                                      - Karl Lehenbauer
#3 lore raw

PATCH 1/1 TPM: remove dead code

Hi Wolfgang,

Quoted 14 lines · Wolfgang Denk

Dear Simon,

Quoted 11 lines · In message <CAPnjgZ0JLarq6r=sHe+cfbjkyuf6hRgGP4BR9u_aSug14MZcrg@mail.gmail.com>…
Quoted 8 lines · Wolfgang Denk

The TPM code was added more than a year or 4 releases ago. This was done under the proposition that board support that would actually use such code would be added soon. However, nothing happened since. The code has no users in mainline, and does not even get build for any configuration, so we cannot even tell if it compiles at all.

Remove the unused code. In in some far future actual users show up, it can be re-added easily.

Quoted 7 lines
Quoted 2 lines

I think you may have missed the pending patches which make use of this. it is important functionality for the Chromebooks (secure boot).

No, I have not missed these. But all the patch does is set CONFIG_GENERIC_LPC_TPM - there is still not a single user defining CONFIG_CMD_TPM, so what does this help? We still have tons of dead code around. Dump it!

So we need a board that defines the command also? I did not realise that was a requirement - certainly I can add that command to the boards also.

Upstreaming the code is a step-by-step process. The TPM is an important component of secure boot, and things have to progress in some sort of fashion. I do understand the dead code argument, but we can't submit high-level code without the drivers it uses (and there are many).

Regards,
Simon

Quoted 11 lines

Best regards,

Wolfgang Denk

--
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
Unix: Some say the learning curve is steep, but you only have to
climb it once. - Karl Lehenbauer

#4 lore raw

PATCH 1/1 TPM: remove dead code

Dear Simon,

Quoted 4 lines · In message <CAPnjgZ0vRbsFWpGqsZPy=TjgJp9r80VnVv2Pcem5werL8bRwmw@mail.gmail.com>…
Quoted 4 lines
Quoted 3 lines

I think you may have missed the pending patches which make use of this. it is important functionality for the Chromebooks (secure boot).

Quoted 8 lines
Quoted 4 lines

No, I have not missed these. But all the patch does is set CONFIG_GENERIC_LPC_TPM - there is still not a single user defining CONFIG_CMD_TPM, so what does this help? We still have tons of dead code around. Dump it!

So we need a board that defines the command also? I did not realise that was a requirement - certainly I can add that command to the boards also.

No, you misunderstand. You stick at the surface, at the formal part of it. Indeed enabling this option for a board would be an improvement - at least then we could be sure that the code compiles. But that does not actually make it _used_. Similar to you enabling of CONFIG_GENERIC_LPC_TPM - the code that needs appears to be just more unused code itself so this is not actual _use_ in the sense of providing a functionality needed for the operation of the device.

Quoted 5 lines

Upstreaming the code is a step-by-step process. The TPM is an important component of secure boot, and things have to progress in some sort of fashion. I do understand the dead code argument, but we can't submit high-level code without the drivers it uses (and there are many).

Well, we had this very same discussion when the TPM code was originally added, and I let myself talk into accepting it, based on the argument that code that needs it is available and will be mainlined "really soon now". More than a year has passed, and _nothing_ happened. Now I see the next wave of patches adding dead code in the same are, and I have to admit that this does not make me enthusiastic.

I think your approach is wrong. You should try and get a minimal version (obviously with very restricted functionality) of your "high-level code" into mainline, and all needed drivers in the same patch series (which makes clear why this should be a minimal configuration). Then add drivers and functionality step by step. Adding a driver here and a driver there one by one with the slight chance that there might - in some unforeseeable future - a board submitted that actually uses these all is nothing I'm going to accept.

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.

Finally, I also have license concerns about the newly submitted code. Statements like "code is governed by a BSD-style license" (in drivers/tpm/tis_i2c.c) are obviusly not acceptabl at all - there are several BSD-style licenses - am I to chose myself any of these, or what???

I have the impression that you are trying to use the end of the merge window to get a whole a bunch of very green code into mainline, ignoring quite a number of rules well known to you.

Best regards,

Wolfgang Denk

Signature · 6 lines
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
Without facts, the decision cannot be made logically. You  must  rely
on your human intuition.
	-- Spock, "Assignment: Earth", stardate unknown
#5 lore raw

PATCH 1/1 TPM: remove dead code

Hi Wolfgang,

Quoted 16 lines · Wolfgang Denk

Dear Simon,

Quoted 4 lines · In message <CAPnjgZ0vRbsFWpGqsZPy=TjgJp9r80VnVv2Pcem5werL8bRwmw@mail.gmail.com>…
Quoted 4 lines
Quoted 3 lines

I think you may have missed the pending patches which make use of this. it is important functionality for the Chromebooks (secure boot).

Quoted 8 lines
Quoted 4 lines

No, I have not missed these. But all the patch does is set CONFIG_GENERIC_LPC_TPM - there is still not a single user defining CONFIG_CMD_TPM, so what does this help? We still have tons of dead code around. Dump it!

So we need a board that defines the command also? I did not realise that was a requirement - certainly I can add that command to the boards also.

Quoted 7 lines

No, you misunderstand. You stick at the surface, at the formal part of it. Indeed enabling this option for a board would be an improvement - at least then we could be sure that the code compiles. But that does not actually make it _used_. Similar to you enabling of CONFIG_GENERIC_LPC_TPM - the code that needs appears to be just more unused code itself so this is not actual _use_ in the sense of providing a functionality needed for the operation of the device.

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.

Quoted 7 lines
Quoted 6 lines

Upstreaming the code is a step-by-step process. The TPM is an important component of secure boot, and things have to progress in some sort of fashion. I do understand the dead code argument, but we can't submit high-level code without the drivers it uses (and there are many).

Quoted 7 lines

Well, we had this very same discussion when the TPM code was originally added, and I let myself talk into accepting it, based on the argument that code that needs it is available and will be mainlined "really soon now". More than a year has passed, and _nothing_ happened. Now I see the next wave of patches adding dead code in the same are, and I have to admit that this does not make me enthusiastic.

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.

Quoted 9 lines

I think your approach is wrong. You should try and get a minimal version (obviously with very restricted functionality) of your "high-level code" into mainline, and all needed drivers in the same patch series (which makes clear why this should be a minimal configuration). Then add drivers and functionality step by step. Adding a driver here and a driver there one by one with the slight chance that there might - in some unforeseeable future - a board submitted that actually uses these all is nothing I'm going to accept.

That's going to be a big series.... but in contrast, display, SATA and GPIO patches are ok, but TPM is not?

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.

Quoted 4 lines

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 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 :-)

Quoted 7 lines

Finally, I also have license concerns about the newly submitted code. Statements like "code is governed by a BSD-style license" (in drivers/tpm/tis_i2c.c) are obviusly not acceptabl at all - there are several BSD-style licenses - am I to chose myself any of these, or what???

Hmmm, that is in one file only, and one that I can easily change without problem. It is not correct - I will change it to GPL.

Quoted 5 lines

I have the impression that you are trying to use the end of the merge window to get a whole a bunch of very green code into mainline, ignoring quite a number of rules well known to you.

Not intentionally. The common/ series includes a few patches that I have carried for quite a while (2 years in some cases), so I thought it was time to either mainline it or drop it. I know from experience that other people's patches can be very useful in certain circumstances even if not perfect. But coding hiding in a dark corner is no use to anyone.

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.

Regards,
Simon

Quoted 12 lines

Best regards,

Wolfgang Denk

--
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 Without facts, the decision cannot be made logically. You must rely on your human intuition.
-- Spock, "Assignment: Earth", stardate unknown

#6 lore raw

PATCH 1/1 TPM: remove dead code

Dear Simon Glass,

Quoted 7 lines · In message <CAPnjgZ2QyRjfjbJTR9MHcsQUEV933oAWPjSTvv-kFrhLCFJRUg@mail.gmail.com>…

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?

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

Quoted 3 lines

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.

Quoted 2 lines

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.

Quoted 4 lines

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.

Quoted 5 lines
Quoted 3 lines

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.

Quoted 3 lines

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.

Quoted 4 lines

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 )

All this discussion confirmed my resolution not to allow for dead code.

Best regards,

Wolfgang Denk

Signature · 6 lines
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
#7 lore raw

PATCH 1/1 TPM: remove dead code

Hi Wolfgang,

Quoted 19 lines · Wolfgang Denk

Dear Simon Glass,

Quoted 7 lines · In message <CAPnjgZ2QyRjfjbJTR9MHcsQUEV933oAWPjSTvv-kFrhLCFJRUg@mail.gmail.com>…

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.

Quoted 8 lines

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.

Quoted 3 lines

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

Enthusiasm is a strong word...

Quoted 5 lines
Quoted 4 lines

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.

Quoted 8 lines

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.

Quoted 8 lines
Quoted 3 lines

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?

Quoted 12 lines
Quoted 5 lines

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".

Quoted 3 lines

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.

Quoted 15 lines
Quoted 6 lines
Quoted 4 lines

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.

Quoted 3 lines

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 :-)

Quoted 7 lines

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.

Quoted 11 lines
Quoted 5 lines

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.

Quoted 8 lines

All this discussion confirmed my resolution not to allow for dead code.

Best regards,

Wolfgang Denk

Regards,
Simon

Quoted 7 lines

--
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

#8 lore raw

PATCH 1/1 TPM: remove dead code

Dear Simon,

Quoted 4 lines · In message <CAPnjgZ0V=MwiVYQ3H-dsVEpNVHMdOPx3Yh3qtPdje0=_Cz=smg@mail.gmail.com>…
Quoted 2 lines

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

Enthusiasm is a strong word...

I tried to find a strictly non-negative phrasing here, because I felt that would be somewhat better ;-)

Quoted 6 lines

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

As soon as I can type on the console something like

	ext2load sata 0:1 200000 boot/uImage

and this is actually capable of reading data from a S-ATA attached storage device, then this is perfectly fine: the driver provides a functionality, which can be used in U-Boot to perform certain actions on the real system.

Quoted 3 lines

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?

If there is a TPM command which actually communicates with the TPM hardware and can be used for example to verify an image checksum or such, then it provides a function.

Quoted 3 lines

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?

This is not what I mean.

Quoted 8 lines

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?

3. would actually not be needed, as long as 1. and 2. really work on some system.

Comment for 1. (but not relateds to this thread, just a note while we are at it): I imagine such functionality should be integrated with the FIT image handling, i. e. like we are today able to verify - say - a sha256 checksum of an image, your code would be able to verify a TPM backed signature. Does this match your plans?

Quoted 1 line

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

Can we agree on something like: code is considered to be "used" (in the sense of it is no dead code) when there is at least one board configuration in mainline where, either as part of the normal initialization sequence or as reaction to some interactive user command, said code will be executed and perform the specific function it was designed for ?

As a side effect this means that there is at least one board configuration that will actually compile and link this piece of code.

Best regards,

Wolfgang Denk

Signature · 4 lines
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
"Free markets select for winning solutions."        - Eric S. Raymond
#9 lore raw

PATCH 1/1 TPM: remove dead code

Hi Wolfgang,

Quoted 10 lines · Wolfgang Denk

Dear Simon,

Quoted 4 lines · In message <CAPnjgZ0V=MwiVYQ3H-dsVEpNVHMdOPx3Yh3qtPdje0=_Cz=smg@mail.gmail.com>…
Quoted 2 lines

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

Enthusiasm is a strong word...

I tried to find a strictly non-negative phrasing here, because I felt that would be somewhat better ;-)

Much appreciated :-)

Quoted 38 lines
Quoted 7 lines

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

As soon as I can type on the console something like

        ext2load sata 0:1 200000 boot/uImage

and this is actually capable of reading data from a S-ATA attached storage device, then this is perfectly fine: the driver provides a functionality, which can be used in U-Boot to perform certain actions on the real system.

Quoted 3 lines

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?

If there is a TPM command which actually communicates with the TPM hardware and can be used for example to verify an image checksum or such, then it provides a function.

Quoted 3 lines

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?

This is not what I mean.

Quoted 7 lines

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

Quoted 4 lines
Quoted 1 line

Does this sound right?

3. would actually not be needed, as long as 1. and 2. really work on some system.

OK I think I understand this now. I will take a look at this and see what I can come up with.

Quoted 6 lines

Comment for 1. (but not relateds to this thread, just a note while we are at it): I imagine such functionality should be integrated with the FIT image handling, i. e. like we are today able to verify - say - a sha256 checksum of an image, your code would be able to verify a TPM backed signature. Does this match your plans?

That would be nice. That format seems almost infinitely flexible so we should be able to make it work.

Still, I believe the FIT image itself needs to be signed. If we try to sign the components of the FIT image, then I think we will get into trouble because it will need multiple crypto operations to verify each piece (and there might be a lot of pieces if we have a lot of FDTs inside). If we try to put the whole-FIT-image signature inside the FIT image, then we have a chicken and egg problem - we cannot sign it until we know what is in it, but we can't know what is in it until we know the signature.

One option would be to embed a FIT image inside a FIT image, like this:

+---- Top Level FIT Image-----------------------------------+
|                                                           |
|    * Hash of embedded FIT image, signed with secret key   |
|                                                           |
|    +---Embedded FIT Image-------------+                   |
|    |                                  |                   |
|    |  kernels                         |                   |
|    |  fdts                            |                   |
|    |  configs                         |                   |
|    |                                  |                   |
|    +----------------------------------+                   |
|                                                           |
+-----------------------------------------------------------+

But this is not ideal because we have to verify the whole image (including the large kernel) just to check its version / compatibility (we may have several kernel options and be trying to pick the best).

Ideally we would be able to split the FIT image into a meta-data piece and a data piece, so we can verify the meta-data all at once with a single key, and the data only as needed when we decide we want to load a particular kernel.

The FDT format sort-of does this, but I believe the kernel is embedded as a blob, so in practice we can't separate the two. Right?

I will think about it some more.

Quoted 11 lines
Quoted 3 lines

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

Can we agree on something like: code is considered to be "used" (in the sense of it is no dead code) when there is at least one board configuration in mainline where, either as part of the normal initialization sequence or as reaction to some interactive user command, said code will be executed and perform the specific function it was designed for ?

Quoted 2 lines

As a side effect this means that there is at least one board configuration that will actually compile and link this piece of code.

Yes that seems reasonable, and it makes sense. But 'reaction to some interactive user command' allows me to create such a command (as I did with TPM) which exercises the driver and thus makes it undead code. This is a bit of a grey area.

Regards,
Simon

Quoted 10 lines

Best regards,

Wolfgang Denk

--
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
"Free markets select for winning solutions." - Eric S. Raymond

#10 lore raw

[U-Boot] Signing Images - was: TPM: remove dead code

[Note: Subject changed, Threading restarted].

Dear Simon,

Quoted 11 lines · In message <CAPnjgZ11wScsnvPG29CFDRYz7Xbtp=WB+tL0UmGvNvHmHmJ-RA@mail.gmail.com>…
Quoted 6 lines

Comment for 1. (but not relateds to this thread, just a note while we are at it): I imagine such functionality should be integrated with the FIT image handling, i. e. like we are today able to verify - say - a sha256 checksum of an image, your code would be able to verify a TPM backed signature. Does this match your plans?

That would be nice. That format seems almost infinitely flexible so we should be able to make it work.

Still, I believe the FIT image itself needs to be signed. If we try to

I don;t think this is a good idea...

Quoted 4 lines

sign the components of the FIT image, then I think we will get into trouble because it will need multiple crypto operations to verify each piece (and there might be a lot of pieces if we have a lot of FDTs inside). ...

Are you sure that would really be a problem?

First, what makes you sure that checking the signature over N parts is so much slower than checking it over one summary image with a much larger size (totalling the sum of the images, plus overhead)? Is setting up such a check really such an expensive operation?

Second, who says you must check the signature of all parts? Even if you have a large number of DT blobs, you will select only one for booting, so it should be sufficient to verify only this one - why should you care about all the other, unused ones? Actually that could be _faster_ than checking everything.

Third, there may be many alternative approaches. For example, you could put all the DT blobs into a small file system (to be used in U-Boot as a RAM disk) - then you can verify just a single file system image, and then just read the needed DT from that. Think a bit longer, and more approaches will spring up.

Quoted 4 lines

... If we try to put the whole-FIT-image signature inside the FIT
image, then we have a chicken and egg problem - we cannot sign it until we know what is in it, but we can't know what is in it until we know the signature.

True.

Quoted 1 line

One option would be to embed a FIT image inside a FIT image, like this:

This does not make much sense to me, too complicated, and I don't see the real need yet.

Quoted 7 lines

Ideally we would be able to split the FIT image into a meta-data piece and a data piece, so we can verify the meta-data all at once with a single key, and the data only as needed when we decide we want to load a particular kernel.

The FDT format sort-of does this, but I believe the kernel is embedded as a blob, so in practice we can't separate the two. Right?

Each "object" carries it's own meta-data, but what an object actually is, is pretty much flexible - of course, the command infrastructre needs to support it. If you want to use "bootm" with the FIT image to boot it directly, then you need to stick with the current format. But of course the bootm command can already now be used as individual sub-commands, and similarly we could (and probably should) add a set of commands (resp. one "fit" command with a set of sub-commands) to perform specific operations on the parts of a FIT image.

Quoted 7 lines
Quoted 2 lines

As a side effect this means that there is at least one board configuration that will actually compile and link this piece of code.

Yes that seems reasonable, and it makes sense. But 'reaction to some interactive user command' allows me to create such a command (as I did with TPM) which exercises the driver and thus makes it undead code. This is a bit of a grey area.

We could try to come up with a lawyer-proof specification of the requirements for such a command (i. e. that it has to perform a "real", or "useful" operation etc.), thus trying to prevent the creation of dummy commands that are ment only to fulfil the "no dead code" requirement. I don't want to go that far. Let's apply common sense. If you write a driver, you probably want to test it some way, so you will have to implement some command interface to exercise the driver in any case. I think there are only few cases where such a command would not be included in the normal U-Boot configuration.

But yes, there are grey areas; but maybe it's enough if we plant big signs "Here Abide Monsters" on such ways...

Best regards,

Wolfgang Denk

Signature · 6 lines
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
"You know, after a woman's raised a family and so on,  she  wants  to
start living her own life."   "Whose life she's _been_ living, then?"
                                  - Terry Pratchett, _Witches Abroad_
#11 lore raw

[U-Boot] Signing Images - was: TPM: remove dead code

Hi Wolfgang,

Quoted 37 lines · Wolfgang Denk

[Note: Subject changed, Threading restarted].

Dear Simon,

Quoted 11 lines · In message <CAPnjgZ11wScsnvPG29CFDRYz7Xbtp=WB+tL0UmGvNvHmHmJ-RA@mail.gmail.com>…
Quoted 6 lines

Comment for 1. (but not relateds to this thread, just a note while we are at it): I imagine such functionality should be integrated with the FIT image handling, i. e. like we are today able to verify - say - a sha256 checksum of an image, your code would be able to verify a TPM backed signature. Does this match your plans?

That would be nice. That format seems almost infinitely flexible so we should be able to make it work.

Still, I believe the FIT image itself needs to be signed. If we try to

I don;t think this is a good idea...

Quoted 4 lines

sign the components of the FIT image, then I think we will get into trouble because it will need multiple crypto operations to verify each piece (and there might be a lot of pieces if we have a lot of FDTs inside). ...

Are you sure that would really be a problem?

First, what makes you sure that checking the signature over N parts is so much slower than checking it over one summary image with a much larger size (totalling the sum of the images, plus overhead)? Is setting up such a check really such an expensive operation?

Second, who says you must check the signature of all parts? Even if you have a large number of DT blobs, you will select only one for booting, so it should be sufficient to verify only this one - why should you care about all the other, unused ones? Actually that could be _faster_ than checking everything.

Quoted 5 lines

Third, there may be many alternative approaches. For example, you could put all the DT blobs into a small file system (to be used in U-Boot as a RAM disk) - then you can verify just a single file system image, and then just read the needed DT from that. Think a bit longer, and more approaches will spring up.

Yes I hope so. For now I think we could do something like sign the kernel and FDTs individually (actually probably sign their hashes since that is likely to be faster) and write the signature in along with the hash.

Quoted 21 lines
Quoted 5 lines

... If we try to put the whole-FIT-image signature inside the FIT
image, then we have a chicken and egg problem - we cannot sign it until we know what is in it, but we can't know what is in it until we know the signature.

True.

Quoted 1 line

One option would be to embed a FIT image inside a FIT image, like this:

This does not make much sense to me, too complicated, and I don't see the real need yet.

Quoted 7 lines

Ideally we would be able to split the FIT image into a meta-data piece and a data piece, so we can verify the meta-data all at once with a single key, and the data only as needed when we decide we want to load a particular kernel.

The FDT format sort-of does this, but I believe the kernel is embedded as a blob, so in practice we can't separate the two. Right?

Quoted 9 lines

Each "object" carries it's own meta-data, but what an object actually is, is pretty much flexible - of course, the command infrastructre needs to support it. If you want to use "bootm" with the FIT image to boot it directly, then you need to stick with the current format. But of course the bootm command can already now be used as individual sub-commands, and similarly we could (and probably should) add a set of commands (resp. one "fit" command with a set of sub-commands) to perform specific operations on the parts of a FIT image.

That sounds good.

I feel that we may still need to sign the whole FIT image, or at least enough to cover the strings and the structure. Otherwise we have a fair bit of code which is looking at user-supplied unverified data. The smaller this 'attach surface' is, the better. But for a starting point I think it is reasonable, and we can perhaps come up with a whole-image signing mechanism later.

Quoted 18 lines
Quoted 7 lines
Quoted 2 lines

As a side effect this means that there is at least one board configuration that will actually compile and link this piece of code.

Yes that seems reasonable, and it makes sense. But 'reaction to some interactive user command' allows me to create such a command (as I did with TPM) which exercises the driver and thus makes it undead code. This is a bit of a grey area.

We could try to come up with a lawyer-proof specification of the requirements for such a command (i. e. that it has to perform a "real", or "useful" operation etc.), thus trying to prevent the creation of dummy commands that are ment only to fulfil the "no dead code" requirement. I don't want to go that far. Let's apply common sense. If you write a driver, you probably want to test it some way, so you will have to implement some command interface to exercise the driver in any case. I think there are only few cases where such a command would not be included in the normal U-Boot configuration.

Quoted 2 lines

But yes, there are grey areas; but maybe it's enough if we plant big signs "Here Abide Monsters" on such ways...

OK.

Regards,
Simon

Quoted 12 lines

Best regards,

Wolfgang Denk

--
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 "You know, after a woman's raised a family and so on, she wants to
start living her own life." "Whose life she's _been_ living, then?"
- Terry Pratchett, _Witches Abroad_

#12 lore raw

PATCH 1/1 TPM: remove dead code

In reply to Wolfgang Denk

Dear Tom,

Quoted 12 lines · In message <[email protected]> you wrote:

The TPM code was added more than a year or 4 releases ago. This was done under the proposition that board support that would actually use such code would be added soon. However, nothing happened since. The code has no users in mainline, and does not even get build for any configuration, so we cannot even tell if it compiles at all.

Remove the unused code. In in some far future actual users show up, it can be re-added easily.

Cc Vadim Bendebury <[email protected]>
Cc Simon Glass <[email protected]>
Quoted 14 lines
---
 README                        |  10 -
 common/Makefile               |   1 -
 common/cmd_tpm.c              | 103 ---------
 doc/driver-model/UDM-tpm.txt  |  48 ----
 drivers/tpm/Makefile          |  43 ----
 drivers/tpm/generic_lpc_tpm.c | 495 ------------------------------------------
 include/tpm.h                 |  71 ------
 7 files changed, 771 deletions(-)
 delete mode 100644 common/cmd_tpm.c
 delete mode 100644 doc/driver-model/UDM-tpm.txt
 delete mode 100644 drivers/tpm/Makefile
 delete mode 100644 drivers/tpm/generic_lpc_tpm.c
 delete mode 100644 include/tpm.h

This was sent before end of the merge window, but has not been applied yet.

Despite some discussion about why having TPM code is generally useful, there are still no actual users for it visible, so I really would like to see this patch applied.

It is trivial to re-activate this code if actual users for it should be added some day to come.

Best regards,

Wolfgang Denk

Signature · 5 lines
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
"I used to think that the brain was the most wonderful  organ  in  my
body. Then I realized who was telling me this."        - Emo Phillips
#13 lore raw

PATCH 1/1 TPM: remove dead code

Hi Wolfgang,

Quoted 33 lines · Wolfgang Denk

Dear Tom,

Quoted 12 lines · In message <[email protected]> you wrote:

The TPM code was added more than a year or 4 releases ago. This was done under the proposition that board support that would actually use such code would be added soon. However, nothing happened since. The code has no users in mainline, and does not even get build for any configuration, so we cannot even tell if it compiles at all.

Remove the unused code. In in some far future actual users show up, it can be re-added easily.

Cc Vadim Bendebury <[email protected]>
Cc Simon Glass <[email protected]>
Quoted 14 lines
---
 README                        |  10 -
 common/Makefile               |   1 -
 common/cmd_tpm.c              | 103 ---------
 doc/driver-model/UDM-tpm.txt  |  48 ----
 drivers/tpm/Makefile          |  43 ----
 drivers/tpm/generic_lpc_tpm.c | 495 ------------------------------------------
 include/tpm.h                 |  71 ------
 7 files changed, 771 deletions(-)
 delete mode 100644 common/cmd_tpm.c
 delete mode 100644 doc/driver-model/UDM-tpm.txt
 delete mode 100644 drivers/tpm/Makefile
 delete mode 100644 drivers/tpm/generic_lpc_tpm.c
 delete mode 100644 include/tpm.h

This was sent before end of the merge window, but has not been applied yet.

Quoted 6 lines

Despite some discussion about why having TPM code is generally useful, there are still no actual users for it visible, so I really would like to see this patch applied.

It is trivial to re-activate this code if actual users for it should be added some day to come.

For what it is worth, this is a NAK from me. There is a patch pending for the next release to enable this for coreboot, and another driver for an i2c TPM driver for the ARM Chromebook also pending, also within the merge window. The TPM support here is light but it is a start, and reverting it seems to serve no purpose (I am happy to maintain the code if it needs work, e.g. for device model). On the contrary this code provides a base on which verified boot can be built, and provides useful functions for talking to a TPM which may be on others boards as well.

Regards,
Simon

Quoted 11 lines

Best regards,

Wolfgang Denk

--
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 "I used to think that the brain was the most wonderful organ in my
body. Then I realized who was telling me this." - Emo Phillips

#14 lore raw

PATCH 1/1 TPM: remove dead code

Hi Wolfgang,

Quoted 43 lines · Simon Glass

Hi Wolfgang,

Quoted 33 lines · Wolfgang Denk

Dear Tom,

Quoted 12 lines · In message <[email protected]> you wrote:

The TPM code was added more than a year or 4 releases ago. This was done under the proposition that board support that would actually use such code would be added soon. However, nothing happened since. The code has no users in mainline, and does not even get build for any configuration, so we cannot even tell if it compiles at all.

Remove the unused code. In in some far future actual users show up, it can be re-added easily.

Cc Vadim Bendebury <[email protected]>
Cc Simon Glass <[email protected]>
Quoted 14 lines
---
 README                        |  10 -
 common/Makefile               |   1 -
 common/cmd_tpm.c              | 103 ---------
 doc/driver-model/UDM-tpm.txt  |  48 ----
 drivers/tpm/Makefile          |  43 ----
 drivers/tpm/generic_lpc_tpm.c | 495 ------------------------------------------
 include/tpm.h                 |  71 ------
 7 files changed, 771 deletions(-)
 delete mode 100644 common/cmd_tpm.c
 delete mode 100644 doc/driver-model/UDM-tpm.txt
 delete mode 100644 drivers/tpm/Makefile
 delete mode 100644 drivers/tpm/generic_lpc_tpm.c
 delete mode 100644 include/tpm.h

This was sent before end of the merge window, but has not been applied yet.

Quoted 6 lines

Despite some discussion about why having TPM code is generally useful, there are still no actual users for it visible, so I really would like to see this patch applied.

It is trivial to re-activate this code if actual users for it should be added some day to come.

Quoted 9 lines

For what it is worth, this is a NAK from me. There is a patch pending for the next release to enable this for coreboot, and another driver for an i2c TPM driver for the ARM Chromebook also pending, also within the merge window. The TPM support here is light but it is a start, and reverting it seems to serve no purpose (I am happy to maintain the code if it needs work, e.g. for device model). On the contrary this code provides a base on which verified boot can be built, and provides useful functions for talking to a TPM which may be on others boards as well.

Part of the reason why the TPM was never turned on in coreboot was the difficulty in upstreaming the code, which resulted in us giving up about a year ago. That effort has since restarted and I am hopeful that it will be successful.

Perhaps as a compromise we could have one more cycle to create a verified boot which uses the TPM drivers, which I think is what is required here? The timing is good at this end to actually do the work.

That would send that right signal that this work is valued upstream. That is a very important signal.

Regards,
SImon

Quoted 15 lines

Regards,
Simon

Quoted 11 lines

Best regards,

Wolfgang Denk

--
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 "I used to think that the brain was the most wonderful organ in my
body. Then I realized who was telling me this." - Emo Phillips

#15 lore raw

PATCH 1/1 TPM: remove dead code

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Quoted 61 lines · Simon Glass

Hi Wolfgang,

Quoted 42 lines · Simon Glass

Hi Wolfgang,

Quoted 31 lines · Wolfgang Denk

Dear Tom,

In message <[email protected]> you

Quoted 11 lines · wrote:

The TPM code was added more than a year or 4 releases ago. This was done under the proposition that board support that would actually use such code would be added soon. However, nothing happened since. The code has no users in mainline, and does not even get build for any configuration, so we cannot even tell if it compiles at all.

Remove the unused code. In in some far future actual users show up, it can be re-added easily.

Quoted 12 lines

Bendebury <[email protected]> Cc: Simon Glass
<[email protected]> --- README | 10 -
common/Makefile | 1 - common/cmd_tpm.c | 103
--------- doc/driver-model/UDM-tpm.txt | 48 ---- drivers/tpm/Makefile | 43 ---- drivers/tpm/generic_lpc_tpm.c | 495 ------------------------------------------ include/tpm.h | 71 ------ 7 files changed, 771 deletions(-) delete mode 100644 common/cmd_tpm.c delete mode 100644 doc/driver-model/UDM-tpm.txt delete mode 100644 drivers/tpm/Makefile delete mode 100644 drivers/tpm/generic_lpc_tpm.c delete mode 100644 include/tpm.h

This was sent before end of the merge window, but has not been applied yet.

Quoted 6 lines

Despite some discussion about why having TPM code is generally useful, there are still no actual users for it visible, so I really would like to see this patch applied.

It is trivial to re-activate this code if actual users for it should be added some day to come.

Quoted 9 lines

For what it is worth, this is a NAK from me. There is a patch pending for the next release to enable this for coreboot, and another driver for an i2c TPM driver for the ARM Chromebook also pending, also within the merge window. The TPM support here is light but it is a start, and reverting it seems to serve no purpose (I am happy to maintain the code if it needs work, e.g. for device model). On the contrary this code provides a base on which verified boot can be built, and provides useful functions for talking to a TPM which may be on others boards as well.

Part of the reason why the TPM was never turned on in coreboot was the difficulty in upstreaming the code, which resulted in us giving up about a year ago. That effort has since restarted and I am hopeful that it will be successful.

Quoted 7 lines

Perhaps as a compromise we could have one more cycle to create a verified boot which uses the TPM drivers, which I think is what is
required here? The timing is good at this end to actually do the
work.

That would send that right signal that this work is valued upstream. That is a very important signal.

I can see where Wolfgang's frustration comes from here. So yes, you've got until v2012.04-rc1 comes out (which will be ~2 weeks after v2013.01 is out) before I pull Wolfgang's patch to have posted a series that makes use of this (which is to say, you have until the end of the merge window for the next release to have something that is mergeable for the release).

- --
Tom

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://www.enigmail.net/

iQIcBAEBAgAGBQJQs8sVAAoJENk4IS6UOR1Wyt0P/2GZMc3RAt/3ZC5SaB/lLVG6 UACxxvWfGWDg7QaYxLokNkbMQfnKNQMGCCrMRbNiU7A5rjbRN+6IeaB7kpQ0CQN3 1SUKcdk2hZSpLXH6KGRg4l7i/0MohKyp1tX2qkLJs4ojwAfbcYO3v+Pz+QIuMcz+ rqURzArPOoHA/7yq4WPkQrB820Y8nelQFYo4w1DJ+cEtZ6c1gWJSLj0LAprj8ZcD DVHYtITA2K3d7SS0dm3TjNwwHXrSuOZ1n/+SQURqQ87nUxRiLdd7W54rPyeB58H7 lRst0AC6y1yPBTL30U898HrWg1owF5r4fTNp+/Qwr2rGG5C4ClnHCkS5vvMYc0J4 hSDPya5z5rv7zQQ4xa2t4lvjZdj+ZQ1hafQfaCJB9+sHs6Qz49MJzKpwLR0cLy6u 9Sl27LskphVXmJ9YosrSlGWjh+RSFoJZhV7hlUpaU3Kvd9qNHafrJvUj8MlCaTuU yFbfO71J88dsIJZlTTSuGQ3NTf5cYNySe91EW7JSGcW4B+IkpTGVb1crxxpzaw0B LTkCT9Sc9AkJezfi4C/a/66bLoaQ3ugtFcsEuk641jiCnlsQvHjNsg6vzgJQ+8NX Cbs5a0lzpUUtFLALyrTakHi6/99SsP20Ez8/p3U9+2FS0xD9XDyiqCf7fxZf48Ek bnafmm4U+kmyhs0i9FzU
=IDf8
-----END PGP SIGNATURE-----

#16 lore raw

PATCH 1/1 TPM: remove dead code

Hi Tom,

Quoted 73 lines · Tom Rini

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Quoted 61 lines · Simon Glass

Hi Wolfgang,

Quoted 42 lines · Simon Glass

Hi Wolfgang,

Quoted 31 lines · Wolfgang Denk

Dear Tom,

In message <[email protected]> you

Quoted 11 lines · wrote:

The TPM code was added more than a year or 4 releases ago. This was done under the proposition that board support that would actually use such code would be added soon. However, nothing happened since. The code has no users in mainline, and does not even get build for any configuration, so we cannot even tell if it compiles at all.

Remove the unused code. In in some far future actual users show up, it can be re-added easily.

Quoted 12 lines

Bendebury <[email protected]> Cc: Simon Glass
<[email protected]> --- README | 10 -
common/Makefile | 1 - common/cmd_tpm.c | 103
--------- doc/driver-model/UDM-tpm.txt | 48 ---- drivers/tpm/Makefile | 43 ---- drivers/tpm/generic_lpc_tpm.c | 495 ------------------------------------------ include/tpm.h | 71 ------ 7 files changed, 771 deletions(-) delete mode 100644 common/cmd_tpm.c delete mode 100644 doc/driver-model/UDM-tpm.txt delete mode 100644 drivers/tpm/Makefile delete mode 100644 drivers/tpm/generic_lpc_tpm.c delete mode 100644 include/tpm.h

This was sent before end of the merge window, but has not been applied yet.

Quoted 6 lines

Despite some discussion about why having TPM code is generally useful, there are still no actual users for it visible, so I really would like to see this patch applied.

It is trivial to re-activate this code if actual users for it should be added some day to come.

Quoted 9 lines

For what it is worth, this is a NAK from me. There is a patch pending for the next release to enable this for coreboot, and another driver for an i2c TPM driver for the ARM Chromebook also pending, also within the merge window. The TPM support here is light but it is a start, and reverting it seems to serve no purpose (I am happy to maintain the code if it needs work, e.g. for device model). On the contrary this code provides a base on which verified boot can be built, and provides useful functions for talking to a TPM which may be on others boards as well.

Part of the reason why the TPM was never turned on in coreboot was the difficulty in upstreaming the code, which resulted in us giving up about a year ago. That effort has since restarted and I am hopeful that it will be successful.

Quoted 7 lines

Perhaps as a compromise we could have one more cycle to create a verified boot which uses the TPM drivers, which I think is what is
required here? The timing is good at this end to actually do the
work.

That would send that right signal that this work is valued upstream. That is a very important signal.

Quoted 6 lines

I can see where Wolfgang's frustration comes from here. So yes, you've got until v2012.04-rc1 comes out (which will be ~2 weeks after v2013.01 is out) before I pull Wolfgang's patch to have posted a series that makes use of this (which is to say, you have until the end of the merge window for the next release to have something that is mergeable for the release).

OK that seems reasonable, it gives me time to make this happen.

Thank you both,
Simon

Quoted 22 lines

- --
Tom

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://www.enigmail.net/

iQIcBAEBAgAGBQJQs8sVAAoJENk4IS6UOR1Wyt0P/2GZMc3RAt/3ZC5SaB/lLVG6 UACxxvWfGWDg7QaYxLokNkbMQfnKNQMGCCrMRbNiU7A5rjbRN+6IeaB7kpQ0CQN3 1SUKcdk2hZSpLXH6KGRg4l7i/0MohKyp1tX2qkLJs4ojwAfbcYO3v+Pz+QIuMcz+ rqURzArPOoHA/7yq4WPkQrB820Y8nelQFYo4w1DJ+cEtZ6c1gWJSLj0LAprj8ZcD DVHYtITA2K3d7SS0dm3TjNwwHXrSuOZ1n/+SQURqQ87nUxRiLdd7W54rPyeB58H7 lRst0AC6y1yPBTL30U898HrWg1owF5r4fTNp+/Qwr2rGG5C4ClnHCkS5vvMYc0J4 hSDPya5z5rv7zQQ4xa2t4lvjZdj+ZQ1hafQfaCJB9+sHs6Qz49MJzKpwLR0cLy6u 9Sl27LskphVXmJ9YosrSlGWjh+RSFoJZhV7hlUpaU3Kvd9qNHafrJvUj8MlCaTuU yFbfO71J88dsIJZlTTSuGQ3NTf5cYNySe91EW7JSGcW4B+IkpTGVb1crxxpzaw0B LTkCT9Sc9AkJezfi4C/a/66bLoaQ3ugtFcsEuk641jiCnlsQvHjNsg6vzgJQ+8NX Cbs5a0lzpUUtFLALyrTakHi6/99SsP20Ez8/p3U9+2FS0xD9XDyiqCf7fxZf48Ek bnafmm4U+kmyhs0i9FzU
=IDf8
-----END PGP SIGNATURE-----