[U-Boot] [PATCH 00/25] dm: Convert TPM drivers to driver model

[Simon Glass]

Hi Christophe,

On 13 August 2015 at 16:52, Simon Glass <sjg at chromium.org> wrote:
> Hi Christophe,
>
> On 13 August 2015 at 14:22, Christophe Ricard
> <christophe.ricard at gmail.com> wrote:
>> 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.ricard at 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_ ?
>
> Let's go with tpm then, at least it is clear.
>
>>
>> Also, we will have TPM 2.0 would it be acceptable to build a second TPM
>> class to support additional features ?
>
> Is it incompatible with the driver API we are talking about (open,
> close, send, recv)?
>
>>>
>>>
>>> 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.
>>
>> [...]

I have a rough crack at this (at u-boot-dm/tpm-working) but have not
had a lot of time for this. It will likely be next week before I tidy
up and resend the patches.

Regards,
Simon