Re: [RFC PATCH] tpm: Workaround failed command reception on Infineon devices

[Stefan Berger <stefanb@linux.ibm.com>]

On 10/3/24 5:59 PM, Stefan Berger wrote:
> 
> 
> On 10/2/24 1:04 PM, Jonathan McDowell wrote:
>> (I'm still in the process of testing this to confirm it fixes the
>> errata I've seen, but I wanted to send it out for comments to make sure
>> it's a reasonable approach.)
>>
>> Some Infineon devices have a issue where the status register will get
>> stuck with a quick REQUEST_USE / COMMAND_READY sequence. The work around
> 
> Did you tell Infineon about it? Maybe they should have a look at their 
> firmware.
> 
> What are the TPMs in your fleet doing? I heared that some TPMs 
> pre-create keys in the background once users requested a few. I would 
> try to create a few primary keys with different combination of key flags 

Actually make this child keys of primary keys:

 > tsscreateprimary
Handle 80000000
 > while :; do time tsscreate -hp 80000000 -si  -opem pubkey.pem ; cat 
pubkey.pem; done

This should give a different key every time and maybe key creation time 
goes up at some point...

> set and in different hierarchies (and flush them) to use up these 
> precreated  keys and see whether that leads to any issues with the TIS 
> responsiveness once presumably the device starts creating keys in the 
> background...
> 
>> is to retry the command submission. Add appropriate logic to do this in
> 
> I would describe it as 'retrying to set the TPM_STS_COMMAND_READY flag 
> on the TIS STS register' because you are not retring a (TPM) command 
> submission, like resubmitting a TPM2_PCR_Extend for example.
> 
> 
> 
>> the send path.
>>
>> Signed-off-by: Jonathan McDowell <noodles@meta.com>
>> ---
>>   drivers/char/tpm/tpm_tis_core.c | 24 +++++++++++++++++++-----
>>   drivers/char/tpm/tpm_tis_core.h |  1 +
>>   include/linux/tpm.h             |  1 +
>>   3 files changed, 21 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/char/tpm/tpm_tis_core.c 
>> b/drivers/char/tpm/tpm_tis_core.c
>> index f6aa0dfadb93..940abd1a868e 100644
>> --- a/drivers/char/tpm/tpm_tis_core.c
>> +++ b/drivers/char/tpm/tpm_tis_core.c
>> @@ -432,16 +432,27 @@ static int tpm_tis_recv(struct tpm_chip *chip, 
>> u8 *buf, size_t count)
>>   static int tpm_tis_send_data(struct tpm_chip *chip, const u8 *buf, 
>> size_t len)
>>   {
>>       struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
>> -    int rc, status, burstcnt;
>> +    int rc, status, burstcnt, retry;
>> +    bool status_fix = test_bit(TPM_TIS_STATUS_WORKAROUND, &priv->flags);
> 
> This should probably be moved to the top.
> int retry = status_fix ? 3 : 1;
> 
>>       size_t count = 0;
>>       bool itpm = test_bit(TPM_TIS_ITPM_WORKAROUND, &priv->flags);
>>       status = tpm_tis_status(chip);
>>       if ((status & TPM_STS_COMMAND_READY) == 0) {
>> -        tpm_tis_ready(chip);
>> -        if (wait_for_tpm_stat
>> -            (chip, TPM_STS_COMMAND_READY, chip->timeout_b,
>> -             &priv->int_queue, false) < 0) {
>> +        retry = status_fix ? 3 : 1;
>> +
>> +        while (retry > 0) {
>> +            tpm_tis_ready(chip);
>> +            if (wait_for_tpm_stat
>> +                (chip, TPM_STS_COMMAND_READY, chip->timeout_b,
>> +                 &priv->int_queue, false) >= 0) {
>> +                break;
>> +            }
>> +
>> +            retry--;
>> +        }
>> +
>> +        if (retry == 0) {
>>               rc = -ETIME;
>>               goto out_err;
>>           }
>> @@ -1147,6 +1158,9 @@ int tpm_tis_core_init(struct device *dev, struct 
>> tpm_tis_data *priv, int irq,
>>           priv->timeout_max = TIS_TIMEOUT_MAX_ATML;
>>       }
>> +    if (priv->manufacturer_id == TPM_VID_IFX)
>> +        set_bit(TPM_TIS_STATUS_WORKAROUND, &priv->flags);
>> +
>>       if (is_bsw()) {
>>           priv->ilb_base_addr = ioremap(INTEL_LEGACY_BLK_BASE_ADDR,
>>                       ILB_REMAP_SIZE);
>> diff --git a/drivers/char/tpm/tpm_tis_core.h 
>> b/drivers/char/tpm/tpm_tis_core.h
>> index 13e99cf65efe..f888da57535d 100644
>> --- a/drivers/char/tpm/tpm_tis_core.h
>> +++ b/drivers/char/tpm/tpm_tis_core.h
>> @@ -89,6 +89,7 @@ enum tpm_tis_flags {
>>       TPM_TIS_INVALID_STATUS        = 1,
>>       TPM_TIS_DEFAULT_CANCELLATION    = 2,
>>       TPM_TIS_IRQ_TESTED        = 3,
>> +    TPM_TIS_STATUS_WORKAROUND    = 4,
>>   };
>>   struct tpm_tis_data {
>> diff --git a/include/linux/tpm.h b/include/linux/tpm.h
>> index 4ee9d13749ad..5f4998626a98 100644
>> --- a/include/linux/tpm.h
>> +++ b/include/linux/tpm.h
>> @@ -272,6 +272,7 @@ enum tpm2_cc_attrs {
>>   #define TPM_VID_WINBOND  0x1050
>>   #define TPM_VID_STM      0x104A
>>   #define TPM_VID_ATML     0x1114
>> +#define TPM_VID_IFX      0x15D1
>>   enum tpm_chip_flags {
>>       TPM_CHIP_FLAG_BOOTSTRAPPED        = BIT(0),
>