Hi Simon,
Thanks for the review and your comments.
Please see mine below:
On 13/08/2015 03:30, Simon Glass wrote:
Hi Christophe,
On 11 August 2015 at 15:50, christophe.ricard
<christophe.ric...@gmail.com> wrote:
Hi Simon,
I pretty much like the move to driver model for TPM.
However, i have some few remarks:
The current i2c driver stick to Infineon TPMs and will not support any other
vendors like ST(in my case).
The main reason for this is that there is no transport protocol over I2C
specification defined by the Trusted Computing Group for TPM1.2.
You can take a look at my release tentative here:
http://lists.denx.de/pipermail/u-boot/2015-August/222596.html
Yes I agree it's probably better to rename it. One more patch...
The tpm.c file was delivering a way to ajust best the waiting time for
command duration to receive a command answer from a TPM command. It was
ported from Linux to u-boot.
You can find in chapter 17 how the table tpm_protected_ordinal_duration,
tpm_ordinal_duration were build.
(https://www.trustedcomputinggroup.org/files/resource_files/E14876A3-1A4B-B294-D086297A1ED38F96/mainP2Structrev103.pdf)
This is defined by the TCG and followed by TPM vendors. In u-boot, this is
used only by Infineon i2c driver but it could/should(?) be used
by all other drivers (i2c and lpc).
In short, the idea is to keep the way TPM commands are transfered giving
the hands to drivers for handling the communication over a specified or
proprietary transport protocol.
I fear the current approach would lead to duplicated codes on may TPM
drivers and 2 very differents kind of drivers (Linux/u-boot) very far from
each other.
In that case we should define what the interface is for the TPM. My
approach is to provide a low-level interface which takes care of
open/close, and sending and receiving bytes.
Since that interface doesn't understand the actual commands it can't
attach different timeouts to each. On the other hand as you say only
one driver uses it.
But since tpm_transmit() currently looks inside the packet, I don't
see why the new xfer() method could not do that also. It removes one
layer of itnerfaces.
I think your approach is acceptable and simplifies the code as well.
I would use the send/recv low level interfaces in tis_xfer reproducing
the tpm_transmit behavior.
Do all TPMs use the same commands and timeouts?
My answer is yes with some few informations or highlights.
When it goes with TPM commands, the litteratures is using term "duration".
Term "timeout" is used for a lower level layer that may not be used by
all TPMs (TIS).
Durations are classified into 3 categories: short, medium and long.
Undefined is for non used Ordinals.
Each categories have a predefined value in the standard but TPM vendors
are free to modified(greater or lower) them according to their
implementation.
Those values can/may be updated using a tpm_getcapability command
requiring at least a prior TPM_Startup.
I believe TPM_Startup and the tpm_getcapability(duration) could be
executed in tis_open.
Just a nitpick, i believe prefix tis_ for the main TPM class functions
may not be appropriate.
What about tcg_ or tpm_ ?
Also, we will have TPM 2.0 would it be acceptable to build a second TPM
class to support additional features ?
In general Linux has ad-hoc interfaces for different things, but in
U-Boot we are trying to standardise on driver model, so normally
function pointers would end up implemented there.
[...]
Regards
Christophe
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
