Re: [PATCH 3/3] Add support for TPM devices over I2C bus

[Ninad Palsule <ninad@linux.vnet.ibm.com>]

On 3/22/23 8:04 AM, Stefan Berger wrote:
>
>
> On 3/22/23 07:50, Stefan Berger wrote:
>>
>>
>> On 3/22/23 07:28, Ninad Palsule wrote:
>>>
>>> On 3/21/23 8:30 PM, Stefan Berger wrote:
>>>>
>
>>>>
>>>> I think there should be tpm_tis_set_data_buffer function that you 
>>>> can call rather than transferring the data byte-by-byte.
>>>>
>>>> Thanks for the series!
>>>>
>>>>   Stefan
>>>
>>> I thought about it but the FIFO case performs multiple operations 
>>> hence I did not want to change it. Currently there is no function to 
>>> set data buffer in the common code.
>>
>> It may not be correct to transfer it in one go, either. I just 
>> printed the I2C specs and I am going to look at them now.
>> When one writes TPM command data to the TIS the STS register has its 
>> TPM_TIS_STS_VALID bit set and TPM_TIS_STS_EXPECT bit reset once the 
>> command is complete. This would imply that you should not have a 
>> holding area for the command bytes but pass them on to the TIS 
>> immediately to get the effect of the STS register...
>
> Regarding the registers defined for the I2C: You can pass the data 
> onto the TIS but you should mask out input flags that are not defined 
> for I2C and if the return value has flags not defined for I2C you 
> should also mask those out as well. This applies to the TPM_INT_ENABLE 
> & TPM_STS registers on read and write and to the TPM_INT_CAPABILITY on 
> read. Also you should implement support for 
> TPM_I2C_INTERACE_CAPABILITY on the I2C layer and return sensible 
> values for the defined bits. The TPM_I2C_DEVICE_ADDRESS register 
> should be handled probably assuming fixed address support only.
>
Good catch.

- Added capability conversion for TPM_I2C_INTERFACE_CAPABILITY.

- Added clearing of bits in TPM_STS register.

- Adde check to reject TPM_I2C_DEVICE_ADDRESS register.

- No changes are required for TPM_INT_ENABLE and TPM_INT_CAPABILITY as 
they have same bits between TPM TIS and TPM I2C.


> Ideally there would be a test case similar to this one here 
> https://github.com/qemu/qemu/blob/master/tests/qtest/tpm-tis-util.c . 
> However, I am not sure how easy it is to talk to I2C without a driver 
> for it.
Ok, Thanks.
>
>   Stefan


Thanks for the review!

Ninad Palsule