On Mon, Jun 08, 2026 at 02:46:28PM +0000, Benoit HOUYERE wrote:
> Hello Serge and Jarkko,
>
>
> We have detected a regression with this fix. Indeed, it miss one zero on
> TPM_LONG_LONG. Initial value was 300000 and not 30000.
>
> > + {TPM2_CC_CREATE_PRIMARY, 30000},
> > + {TPM2_CC_CREATE, 30000},
> > + {TPM2_CC_CREATE_LOADED, 30000},
>
> > +enum tpm2_durations {
> > TPM2_DURATION_SHORT = 20,
> > - TPM2_DURATION_MEDIUM = 750,
> > TPM2_DURATION_LONG = 2000,
> > - TPM2_DURATION_LONG_LONG = 300000,
> > TPM2_DURATION_DEFAULT = 120000,
> > };
>
> Best Regards, Cordialement, Cordialmente, Hälsningar, 最好的问候, Mit besten
> Grüßen, 真心を込めて, 진심으로
>
>
> Benoit HOUYERE | Tel: +33 6 14 22 81 30
> TPM specialist
This has been fixed in 478a3f949a43822dc6ce089345ae80e8dcde3300.
>
> -----Original Message-----
> From: Serge E. Hallyn <[email protected]>
> Sent: Friday, September 19, 2025 5:49 AM
> To: Jarkko Sakkinen <[email protected]>
> Cc: [email protected]; Frédéric Jouen <[email protected]>;
> Peter Huewe <[email protected]>; Jason Gunthorpe <[email protected]>; James
> Bottomley <[email protected]>; Mimi Zohar
> <[email protected]>; David Howells <[email protected]>; Paul Moore
> <[email protected]>; James Morris <[email protected]>; Serge E. Hallyn
> <[email protected]>; open list <[email protected]>; open
> list:KEYS-TRUSTED <[email protected]>; open list:SECURITY SUBSYSTEM
> <[email protected]>
> Subject: Re: [PATCH v2] tpm: use a map for tpm2_calc_ordinal_duration()
>
> On Thu, Sep 18, 2025 at 10:30:18PM +0300, Jarkko Sakkinen wrote:
> > The current shenanigans for duration calculation introduce too much
> > complexity for a trivial problem, and further the code is hard to
> > patch and maintain.
> >
> > Address these issues with a flat look-up table, which is easy to
> > understand and patch. If leaf driver specific patching is required in
> > future, it is easy enough to make a copy of this table during driver
> > initialization and add the chip parameter back.
> >
> > 'chip->duration' is retained for TPM 1.x.
> >
> > As the first entry for this new behavior address TCG spec update
> > mentioned in this issue:
> >
> > https://github.com/raspberrypi/linux/issues/7054
> >
> > Therefore, for TPM_SelfTest the duration is set to 3000 ms.
> >
> > This does not categorize a as bug, given that this is introduced to
> > the spec after the feature was originally made.
> >
> > Cc: Frédéric Jouen <[email protected]>
> > Signed-off-by: Jarkko Sakkinen <[email protected]>
>
> fwiw (which shouldn't be much) looks good to me, but two questions, one here
> and one below.
>
> First, it looks like in the existing code it is possible for a tpm2 chip to
> set its own timeouts and then set the TPM_CHIP_FLAG_HAVE_TIMEOUTS flag to
> avoid using the defaults, but I don't see anything using that in-tree. Is it
> possible that there are out of tree drivers that will be sabotaged here? Or
> am I misunderstanding that completely?
>
> > ---
> > v2:
> > - Add the missing msec_to_jiffies() calls.
> > - Drop redundant stuff.
> > ---
> > drivers/char/tpm/tpm-interface.c | 2 +-
> > drivers/char/tpm/tpm.h | 2 +-
> > drivers/char/tpm/tpm2-cmd.c | 127 ++++++++-----------------------
> > include/linux/tpm.h | 5 +-
> > 4 files changed, 37 insertions(+), 99 deletions(-)
> >
> > diff --git a/drivers/char/tpm/tpm-interface.c
> > b/drivers/char/tpm/tpm-interface.c
> > index b71725827743..c9f173001d0e 100644
> > --- a/drivers/char/tpm/tpm-interface.c
> > +++ b/drivers/char/tpm/tpm-interface.c
> > @@ -52,7 +52,7 @@ MODULE_PARM_DESC(suspend_pcr, unsigned long
> > tpm_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal) {
> > if (chip->flags & TPM_CHIP_FLAG_TPM2)
> > - return tpm2_calc_ordinal_duration(chip, ordinal);
> > + return tpm2_calc_ordinal_duration(ordinal);
> > else
> > return tpm1_calc_ordinal_duration(chip, ordinal); } diff --git
> > a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h index
> > 7bb87fa5f7a1..2726bd38e5ac 100644
> > --- a/drivers/char/tpm/tpm.h
> > +++ b/drivers/char/tpm/tpm.h
> > @@ -299,7 +299,7 @@ ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32
> > property_id, ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip);
> > int tpm2_auto_startup(struct tpm_chip *chip); void
> > tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type); -unsigned
> > long tpm2_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);
> > +unsigned long tpm2_calc_ordinal_duration(u32 ordinal);
> > int tpm2_probe(struct tpm_chip *chip); int
> > tpm2_get_cc_attrs_tbl(struct tpm_chip *chip); int tpm2_find_cc(struct
> > tpm_chip *chip, u32 cc); diff --git a/drivers/char/tpm/tpm2-cmd.c
> > b/drivers/char/tpm/tpm2-cmd.c index 524d802ede26..7d77f6fbc152 100644
> > --- a/drivers/char/tpm/tpm2-cmd.c
> > +++ b/drivers/char/tpm/tpm2-cmd.c
> > @@ -28,120 +28,57 @@ static struct tpm2_hash tpm2_hash_map[] = {
> >
> > int tpm2_get_timeouts(struct tpm_chip *chip) {
> > - /* Fixed timeouts for TPM2 */
> > chip->timeout_a = msecs_to_jiffies(TPM2_TIMEOUT_A);
> > chip->timeout_b = msecs_to_jiffies(TPM2_TIMEOUT_B);
> > chip->timeout_c = msecs_to_jiffies(TPM2_TIMEOUT_C);
> > chip->timeout_d = msecs_to_jiffies(TPM2_TIMEOUT_D);
> > -
> > - /* PTP spec timeouts */
> > - chip->duration[TPM_SHORT] = msecs_to_jiffies(TPM2_DURATION_SHORT);
> > - chip->duration[TPM_MEDIUM] = msecs_to_jiffies(TPM2_DURATION_MEDIUM);
> > - chip->duration[TPM_LONG] = msecs_to_jiffies(TPM2_DURATION_LONG);
> > -
> > - /* Key creation commands long timeouts */
> > - chip->duration[TPM_LONG_LONG] =
> > - msecs_to_jiffies(TPM2_DURATION_LONG_LONG);
> > -
> > chip->flags |= TPM_CHIP_FLAG_HAVE_TIMEOUTS;
> > -
> > return 0;
> > }
> >
> > -/**
> > - * tpm2_ordinal_duration_index() - returns an index to the chip
> > duration table
> > - * @ordinal: TPM command ordinal.
> > - *
> > - * The function returns an index to the chip duration table
> > - * (enum tpm_duration), that describes the maximum amount of
> > - * time the chip could take to return the result for a particular ordinal.
> > - *
> > - * The values of the MEDIUM, and LONG durations are taken
> > - * from the PC Client Profile (PTP) specification (750, 2000 msec)
> > - *
> > - * LONG_LONG is for commands that generates keys which empirically
> > takes
> > - * a longer time on some systems.
> > - *
> > - * Return:
> > - * * TPM_MEDIUM
> > - * * TPM_LONG
> > - * * TPM_LONG_LONG
> > - * * TPM_UNDEFINED
> > +/*
> > + * Contains the maximum durations in milliseconds for TPM2 commands.
> > */
> > -static u8 tpm2_ordinal_duration_index(u32 ordinal) -{
> > - switch (ordinal) {
> > - /* Startup */
> > - case TPM2_CC_STARTUP: /* 144 */
> > - return TPM_MEDIUM;
> > -
> > - case TPM2_CC_SELF_TEST: /* 143 */
> > - return TPM_LONG;
> > -
> > - case TPM2_CC_GET_RANDOM: /* 17B */
> > - return TPM_LONG;
> > -
> > - case TPM2_CC_SEQUENCE_UPDATE: /* 15C */
> > - return TPM_MEDIUM;
> > - case TPM2_CC_SEQUENCE_COMPLETE: /* 13E */
> > - return TPM_MEDIUM;
> > - case TPM2_CC_EVENT_SEQUENCE_COMPLETE: /* 185 */
> > - return TPM_MEDIUM;
> > - case TPM2_CC_HASH_SEQUENCE_START: /* 186 */
> > - return TPM_MEDIUM;
> > -
> > - case TPM2_CC_VERIFY_SIGNATURE: /* 177 */
> > - return TPM_LONG_LONG;
> > -
> > - case TPM2_CC_PCR_EXTEND: /* 182 */
> > - return TPM_MEDIUM;
> > -
> > - case TPM2_CC_HIERARCHY_CONTROL: /* 121 */
> > - return TPM_LONG;
> > - case TPM2_CC_HIERARCHY_CHANGE_AUTH: /* 129 */
> > - return TPM_LONG;
> > -
> > - case TPM2_CC_GET_CAPABILITY: /* 17A */
> > - return TPM_MEDIUM;
> > -
> > - case TPM2_CC_NV_READ: /* 14E */
> > - return TPM_LONG;
> > -
> > - case TPM2_CC_CREATE_PRIMARY: /* 131 */
> > - return TPM_LONG_LONG;
> > - case TPM2_CC_CREATE: /* 153 */
> > - return TPM_LONG_LONG;
> > - case TPM2_CC_CREATE_LOADED: /* 191 */
> > - return TPM_LONG_LONG;
> > -
> > - default:
> > - return TPM_UNDEFINED;
> > - }
> > -}
> > +static const struct {
> > + unsigned long ordinal;
> > + unsigned long duration;
> > +} tpm2_ordinal_duration_map[] = {
> > + {TPM2_CC_STARTUP, 750},
> > + {TPM2_CC_SELF_TEST, 3000},
>
> I assume you intended to increase TPM2_CC_SELF_TEST from 2000 to 3000 here?
> But it's not mentioned in the commit, so making sure...
>
> > + {TPM2_CC_GET_RANDOM, 2000},
> > + {TPM2_CC_SEQUENCE_UPDATE, 750},
> > + {TPM2_CC_SEQUENCE_COMPLETE, 750},
> > + {TPM2_CC_EVENT_SEQUENCE_COMPLETE, 750},
> > + {TPM2_CC_HASH_SEQUENCE_START, 750},
> > + {TPM2_CC_VERIFY_SIGNATURE, 30000},
> > + {TPM2_CC_PCR_EXTEND, 750},
> > + {TPM2_CC_HIERARCHY_CONTROL, 2000},
> > + {TPM2_CC_HIERARCHY_CHANGE_AUTH, 2000},
> > + {TPM2_CC_GET_CAPABILITY, 750},
> > + {TPM2_CC_NV_READ, 2000},
> > + {TPM2_CC_CREATE_PRIMARY, 30000},
> > + {TPM2_CC_CREATE, 30000},
> > + {TPM2_CC_CREATE_LOADED, 30000},
> > +};
> >
> > /**
> > - * tpm2_calc_ordinal_duration() - calculate the maximum command duration
> > - * @chip: TPM chip to use.
> > + * tpm2_calc_ordinal_duration() - Calculate the maximum command
> > + duration
> > * @ordinal: TPM command ordinal.
> > *
> > - * The function returns the maximum amount of time the chip could
> > take
> > - * to return the result for a particular ordinal in jiffies.
> > - *
> > - * Return: A maximal duration time for an ordinal in jiffies.
> > + * Returns the maximum amount of time the chip is expected by kernel
> > + to
> > + * take in jiffies.
> > */
> > -unsigned long tpm2_calc_ordinal_duration(struct tpm_chip *chip, u32
> > ordinal)
> > +unsigned long tpm2_calc_ordinal_duration(u32 ordinal)
> > {
> > - unsigned int index;
> > + int i;
> >
> > - index = tpm2_ordinal_duration_index(ordinal);
> > + for (i = 0; i < ARRAY_SIZE(tpm2_ordinal_duration_map); i++)
> > + if (ordinal == tpm2_ordinal_duration_map[i].ordinal)
> > + return
> > msecs_to_jiffies(tpm2_ordinal_duration_map[i].duration);
> >
> > - if (index != TPM_UNDEFINED)
> > - return chip->duration[index];
> > - else
> > - return msecs_to_jiffies(TPM2_DURATION_DEFAULT);
> > + return msecs_to_jiffies(TPM2_DURATION_DEFAULT);
> > }
> >
> > -
> > struct tpm2_pcr_read_out {
> > __be32 update_cnt;
> > __be32 pcr_selects_cnt;
> > diff --git a/include/linux/tpm.h b/include/linux/tpm.h index
> > b0e9eb5ef022..dc0338a783f3 100644
> > --- a/include/linux/tpm.h
> > +++ b/include/linux/tpm.h
> > @@ -228,10 +228,11 @@ enum tpm2_timeouts {
> > TPM2_TIMEOUT_B = 4000,
> > TPM2_TIMEOUT_C = 200,
> > TPM2_TIMEOUT_D = 30,
> > +};
> > +
> > +enum tpm2_durations {
> > TPM2_DURATION_SHORT = 20,
> > - TPM2_DURATION_MEDIUM = 750,
> > TPM2_DURATION_LONG = 2000,
> > - TPM2_DURATION_LONG_LONG = 300000,
> > TPM2_DURATION_DEFAULT = 120000,
> > };
> >
> > --
> > 2.39.5
>
BR, Jarkko
