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

[Stefan Berger]

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@xxxxxxxx>
> > ---
> >   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),