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

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

Hi Joel,

On 3/26/23 8:05 PM, Joel Stanley wrote:
> Hi Ninad,
>
> On Sun, 26 Mar 2023 at 22:44, Ninad Palsule <ninad@linux.ibm.com> wrote:
>> Hello,
>>
>> I have incorporated review comments from Stefan. Please review.
>>
>> This drop adds support for the TPM devices attached to the I2C bus. It
>> only supports the TPM2 protocol. You need to run it with the external
>> TPM emulator like swtpm. I have tested it with swtpm.
> Nice work. I tested these stop cedric's aspeed-8.0 qemu tree, using
> the rainier machine and the openbmc dev-6.1 kernel.
>
> We get this message when booting from a kernel:
>
> [    0.582699] tpm_tis_i2c 12-002e: 2.0 TPM (device-id 0x1, rev-id 1)
> [    0.586361] tpm tpm0: A TPM error (256) occurred attempting the self test
> [    0.586623] tpm tpm0: starting up the TPM manually
>
> Do we understand why the error appears?


Yes, As per kernel code this is an expected error for some emulators.

On swtpm emulator, It returns TPM2_RC_INITIALIZE if emulator is not 
initialized. I searched it in swtpm and it indicated that selftest 
requested before it is initialized. I meant to ask Stefan but busy with 
the review comments.

This function comment in the driver mentioned below indicate that this 
case possible with emulators.

/**
  * tpm2_startup - turn on the TPM
  * @chip: TPM chip to use
  *
  * Normally the firmware should start the TPM. This function is 
provided as a
  * workaround if this does not happen. A legal case for this could be for
  * example when a TPM emulator is used.
  *
  * Return: same as tpm_transmit_cmd()
  */

static int tpm2_startup(struct tpm_chip *chip)


>
> # grep -r . /sys/class/tpm/tpm0/pcr-sha256/ | sort -n -k 7 -t /
> /sys/class/tpm/tpm0/pcr-sha256/0:0000000000000000000000000000000000000000000000000000000000000000
> /sys/class/tpm/tpm0/pcr-sha256/1:0000000000000000000000000000000000000000000000000000000000000000
> /sys/class/tpm/tpm0/pcr-sha256/2:0000000000000000000000000000000000000000000000000000000000000000
> /sys/class/tpm/tpm0/pcr-sha256/3:0000000000000000000000000000000000000000000000000000000000000000
> /sys/class/tpm/tpm0/pcr-sha256/4:0000000000000000000000000000000000000000000000000000000000000000
> /sys/class/tpm/tpm0/pcr-sha256/5:0000000000000000000000000000000000000000000000000000000000000000
> /sys/class/tpm/tpm0/pcr-sha256/6:0000000000000000000000000000000000000000000000000000000000000000
> /sys/class/tpm/tpm0/pcr-sha256/7:0000000000000000000000000000000000000000000000000000000000000000
> /sys/class/tpm/tpm0/pcr-sha256/8:0000000000000000000000000000000000000000000000000000000000000000
> /sys/class/tpm/tpm0/pcr-sha256/9:0000000000000000000000000000000000000000000000000000000000000000
> /sys/class/tpm/tpm0/pcr-sha256/10:0000000000000000000000000000000000000000000000000000000000000000
> /sys/class/tpm/tpm0/pcr-sha256/11:0000000000000000000000000000000000000000000000000000000000000000
> /sys/class/tpm/tpm0/pcr-sha256/12:0000000000000000000000000000000000000000000000000000000000000000
> /sys/class/tpm/tpm0/pcr-sha256/13:0000000000000000000000000000000000000000000000000000000000000000
> /sys/class/tpm/tpm0/pcr-sha256/14:0000000000000000000000000000000000000000000000000000000000000000
> /sys/class/tpm/tpm0/pcr-sha256/15:0000000000000000000000000000000000000000000000000000000000000000
> /sys/class/tpm/tpm0/pcr-sha256/16:0000000000000000000000000000000000000000000000000000000000000000
> /sys/class/tpm/tpm0/pcr-sha256/17:FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF
> /sys/class/tpm/tpm0/pcr-sha256/18:FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF
> /sys/class/tpm/tpm0/pcr-sha256/19:FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF
> /sys/class/tpm/tpm0/pcr-sha256/20:FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF
> /sys/class/tpm/tpm0/pcr-sha256/21:FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF
> /sys/class/tpm/tpm0/pcr-sha256/22:FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF
> /sys/class/tpm/tpm0/pcr-sha256/23:0000000000000000000000000000000000000000000000000000000000000000
>
> If I boot through the openbmc u-boot for the p10bmc machine, which
> measures things into the PCRs:
>
> [    0.556713] tpm_tis_i2c 12-002e: 2.0 TPM (device-id 0x1, rev-id 1)
>
> / # grep -r . /sys/class/tpm/tpm0/pcr-sha256/ | sort -n -k 7 -t /
> /sys/class/tpm/tpm0/pcr-sha256/0:AFA13691EFC7BC6E189E92347F20676FB4523302CB957DA9A65C3430C45E8BCC
> /sys/class/tpm/tpm0/pcr-sha256/1:37F0F710A5502FAE6DB7433B36001FEE1CBF15BA2A7D6923207FF56888584714
> /sys/class/tpm/tpm0/pcr-sha256/2:E21B703EE69C77476BCCB43EC0336A9A1B2914B378944F7B00A10214CA8FEA93
> /sys/class/tpm/tpm0/pcr-sha256/3:E21B703EE69C77476BCCB43EC0336A9A1B2914B378944F7B00A10214CA8FEA93
> /sys/class/tpm/tpm0/pcr-sha256/4:E21B703EE69C77476BCCB43EC0336A9A1B2914B378944F7B00A10214CA8FEA93
> /sys/class/tpm/tpm0/pcr-sha256/5:E21B703EE69C77476BCCB43EC0336A9A1B2914B378944F7B00A10214CA8FEA93
> /sys/class/tpm/tpm0/pcr-sha256/6:E21B703EE69C77476BCCB43EC0336A9A1B2914B378944F7B00A10214CA8FEA93
> /sys/class/tpm/tpm0/pcr-sha256/7:E21B703EE69C77476BCCB43EC0336A9A1B2914B378944F7B00A10214CA8FEA93
> /sys/class/tpm/tpm0/pcr-sha256/8:AE67485BD01E8D6FE0208C46C473940173F66E9C6F43C75ABB404375787E9705
> /sys/class/tpm/tpm0/pcr-sha256/9:DB99D92EADBB446894CB0C062AEB673F60DDAFBC62BC2A9CA561A13B31E5357C
> /sys/class/tpm/tpm0/pcr-sha256/10:0000000000000000000000000000000000000000000000000000000000000000
> /sys/class/tpm/tpm0/pcr-sha256/11:0000000000000000000000000000000000000000000000000000000000000000
> /sys/class/tpm/tpm0/pcr-sha256/12:0000000000000000000000000000000000000000000000000000000000000000
> /sys/class/tpm/tpm0/pcr-sha256/13:0000000000000000000000000000000000000000000000000000000000000000
> /sys/class/tpm/tpm0/pcr-sha256/14:0000000000000000000000000000000000000000000000000000000000000000
> /sys/class/tpm/tpm0/pcr-sha256/15:0000000000000000000000000000000000000000000000000000000000000000
> /sys/class/tpm/tpm0/pcr-sha256/16:0000000000000000000000000000000000000000000000000000000000000000
> /sys/class/tpm/tpm0/pcr-sha256/17:FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF
> /sys/class/tpm/tpm0/pcr-sha256/18:FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF
> /sys/class/tpm/tpm0/pcr-sha256/19:FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF
> /sys/class/tpm/tpm0/pcr-sha256/20:FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF
> /sys/class/tpm/tpm0/pcr-sha256/21:FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF
> /sys/class/tpm/tpm0/pcr-sha256/22:FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF
> /sys/class/tpm/tpm0/pcr-sha256/23:0000000000000000000000000000000000000000000000000000000000000000

Great thanks. I could not try it.


>
> However on a clean boot into the TPM, the u-boot tpm commands fail:
>
> ast# tpm info
> tpm@2e v2.0: VendorID 0x1014, DeviceID 0x0001, RevisionID 0x01 [closed]
> ast# tpINTERRUPT>
> ast# tpm init
> ast# tpm info
> tpm@2e v2.0: VendorID 0x1014, DeviceID 0x0001, RevisionID 0x01 [open]
> ast# tpm pcr_read 0 0x81000000
> Error: 256
> ast# md.l 0x81000000 16
> 81000000: 00000000 00000000 00000000 00000000    ................
> 81000010: 00000000 00000000 00000000 00000000    ................
> 81000020: 00000000 00000000 00000000 00000000    ................
> 81000030: 00000000 00000000 00000000 00000000    ................
> 81000040: 00000000 00000000 00000000 00000000    ................
> 81000050: 00000000 00000000                      ........
>
> This doesn't need to block merging into qemu, as the model works fine
> for pcr measurement and accessing under Linux. However it would be
> good to work though these issues in case there's a modelling
> discrepancy.


Yes, Please provide me details on how to reproduce it. I will take a look.


Thanks for the review.

Ninad

>
>
>
>> I have refered to the work done by zhdaniel@meta.com but at the core
>> level out implementation is different.
>> https://github.com/theopolis/qemu/commit/2e2e57cde9e419c36af8071bb85392ad1ed70966
>>
>> Based-on: $MESSAGE_ID
>>
>>
>> Ninad Palsule (3):
>>    docs: Add support for TPM devices over I2C bus
>>    tpm: Extend common APIs to support TPM TIS I2C
>>    tpm: Add support for TPM device over I2C bus
>>
>>   docs/specs/tpm.rst      |  32 +++
>>   hw/arm/Kconfig          |   1 +
>>   hw/tpm/Kconfig          |   7 +
>>   hw/tpm/meson.build      |   1 +
>>   hw/tpm/tpm_tis.h        |   3 +
>>   hw/tpm/tpm_tis_common.c |  36 ++-
>>   hw/tpm/tpm_tis_i2c.c    | 540 ++++++++++++++++++++++++++++++++++++++++
>>   hw/tpm/trace-events     |   6 +
>>   include/hw/acpi/tpm.h   |  31 +++
>>   include/sysemu/tpm.h    |   3 +
>>   10 files changed, 652 insertions(+), 8 deletions(-)
>>   create mode 100644 hw/tpm/tpm_tis_i2c.c
>>
>> --
>> 2.37.2
>>