Hi Simon,

On 13/08/2015 03:30, Simon Glass wrote:
Hi Christophe,

On 11 August 2015 at 15:42, christophe.ricard
<christophe.ric...@gmail.com> wrote:
Hi Simon,

I would basically disagree with this one.
The code from tpm.c you are merging into tpm_tis_i2c may not only be used by
tpm_tis_i2c as it is using data from TPM standards (e.g: Trusted computing
group) that should be used by other TPM drivers.
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 come from a Linux port.

As a result, we can imagine tis_sendrecv as a generic function where
tpm_transmit manage the way tpm commands are sent following specification
giving the hand to drivers thanks to the tpm_vendor_specific field
{send,recv} for handling the communication over a specified or proprietary
transport protocol.
As an example in tpm_tis_lpc, a 1s command duration might be too short or
too long for some commands and might be really hard to debug in case someone
decide to had a new TPM command support in u-boot.
Also 1s might be enought for the current commands or for evaluated TPM but
it may require a longer duration for some other.
By reading the duration TPM capability, that will be just generic.
But this code is only used by the Infineon driver.
My idea is to standardize the way a tpm does a command transfer in u-boot.
The approach is applicable to all the existing u-boot tpm drivers.

Also tpm_tis_i2c is Infineon specific and does not fit to any TCG standard
for TPM1.2, i would recommand to rename this driver tpm_i2c_infineon to be
inline with Linux naming and not confuse a future user on what it support.
Yes we should do that.

At the end from my delivery tentative, a Linux port as well, you may see
that i also rely on those functions. However I am not doing any check on the
command duration. This is to be improved...

May you prefer an approach that would not lead to duplicated code ?
I wonder if the timing of each command can be attached to the uclass
and handled there. We might have a function like:

tpm_xfer()

which sends data to the TPM and receives a rseponse. This can be
implemented by the uclass.

Then the driver interface (which I currently have as xfer()) could be
send() and receive(). The time delay measurement could happen in the
uclass.

So in summary:

tpm-uclass.c:
tpm_xfer()
    - calls driver->send()
    - checks timeout based on data supplied by the driver
     -calls driver->receive()
    - checks timeout based on data supplied by the driver
     - returns result

Then the drivers only need to implement simple send and receive functions.
I agree with this approach.

[...]

Best REgards
Christophe
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to