* [PATCH v2] tpm: use a map for tpm2_calc_ordinal_duration() @ 2025-09-18 19:30 Jarkko Sakkinen 2025-09-18 19:37 ` Jarkko Sakkinen 2025-09-19 3:49 ` Serge E. Hallyn 0 siblings, 2 replies; 7+ messages in thread From: Jarkko Sakkinen @ 2025-09-18 19:30 UTC (permalink / raw) To: linux-integrity Cc: Jarkko Sakkinen, Frédéric Jouen, Peter Huewe, Jason Gunthorpe, James Bottomley, Mimi Zohar, David Howells, Paul Moore, James Morris, Serge E. Hallyn, open list, open list:KEYS-TRUSTED, open list:SECURITY SUBSYSTEM 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 <fjouen@sealsq.com> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org> --- 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}, + {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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2] tpm: use a map for tpm2_calc_ordinal_duration() 2025-09-18 19:30 [PATCH v2] tpm: use a map for tpm2_calc_ordinal_duration() Jarkko Sakkinen @ 2025-09-18 19:37 ` Jarkko Sakkinen 2025-09-19 3:49 ` Serge E. Hallyn 1 sibling, 0 replies; 7+ messages in thread From: Jarkko Sakkinen @ 2025-09-18 19:37 UTC (permalink / raw) To: linux-integrity Cc: Frédéric Jouen, Peter Huewe, Jason Gunthorpe, James Bottomley, Mimi Zohar, David Howells, Paul Moore, James Morris, Serge E. Hallyn, open list, open list:KEYS-TRUSTED, open list:SECURITY SUBSYSTEM 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 <fjouen@sealsq.com> > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org> > --- > v2: > - Add the missing msec_to_jiffies() calls. > - Drop redundant stuff. Run also through kselftest. BR, Jarkko ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] tpm: use a map for tpm2_calc_ordinal_duration() 2025-09-18 19:30 [PATCH v2] tpm: use a map for tpm2_calc_ordinal_duration() Jarkko Sakkinen 2025-09-18 19:37 ` Jarkko Sakkinen @ 2025-09-19 3:49 ` Serge E. Hallyn 2025-09-19 7:05 ` Jarkko Sakkinen 2026-06-08 14:46 ` Benoit HOUYERE 1 sibling, 2 replies; 7+ messages in thread From: Serge E. Hallyn @ 2025-09-19 3:49 UTC (permalink / raw) To: Jarkko Sakkinen Cc: linux-integrity, Frédéric Jouen, Peter Huewe, Jason Gunthorpe, James Bottomley, Mimi Zohar, David Howells, Paul Moore, James Morris, Serge E. Hallyn, open list, open list:KEYS-TRUSTED, open list:SECURITY SUBSYSTEM 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 <fjouen@sealsq.com> > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org> 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] tpm: use a map for tpm2_calc_ordinal_duration() 2025-09-19 3:49 ` Serge E. Hallyn @ 2025-09-19 7:05 ` Jarkko Sakkinen 2025-09-19 14:47 ` Serge E. Hallyn 2026-06-08 14:46 ` Benoit HOUYERE 1 sibling, 1 reply; 7+ messages in thread From: Jarkko Sakkinen @ 2025-09-19 7:05 UTC (permalink / raw) To: Serge E. Hallyn Cc: linux-integrity, Frédéric Jouen, Peter Huewe, Jason Gunthorpe, James Bottomley, Mimi Zohar, David Howells, Paul Moore, James Morris, open list, open list:KEYS-TRUSTED, open list:SECURITY SUBSYSTEM On Thu, Sep 18, 2025 at 10:49:28PM -0500, Serge E. Hallyn wrote: > 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 <fjouen@sealsq.com> > > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org> > > 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? Good questions, and I can brief a bit about the context of the pre-existing art and this change. This complexity was formed in 2014 when I originally developed TPM2 support and the only available testing plaform was early Intel PTT with a flakky version of TPM2 support (e.g., no localities). Since then we haven't had per leaf-driver divergence. Further, I think that this type of layout is actually a better fit if we ever need to quirks for command durations for a particular device, as then we can migrate to "copy and patch" semantics i.e., have a copy of this map in the chip structure. As per out-of-tree drivers, it's unfortunate reality of out-of-tree drivers :-) However, this will definitely add some extra work, when backporting fixes (not overwhelmingly much). BR, Jarkko ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] tpm: use a map for tpm2_calc_ordinal_duration() 2025-09-19 7:05 ` Jarkko Sakkinen @ 2025-09-19 14:47 ` Serge E. Hallyn 0 siblings, 0 replies; 7+ messages in thread From: Serge E. Hallyn @ 2025-09-19 14:47 UTC (permalink / raw) To: Jarkko Sakkinen Cc: Serge E. Hallyn, linux-integrity, Frédéric Jouen, Peter Huewe, Jason Gunthorpe, James Bottomley, Mimi Zohar, David Howells, Paul Moore, James Morris, open list, open list:KEYS-TRUSTED, open list:SECURITY SUBSYSTEM On Fri, Sep 19, 2025 at 10:05:58AM +0300, Jarkko Sakkinen wrote: > On Thu, Sep 18, 2025 at 10:49:28PM -0500, Serge E. Hallyn wrote: > > 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. D'oh! It *was* in the commit message all along, sorry. > > > 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 <fjouen@sealsq.com> > > > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org> Looks good, thank you. Reviewed-by: Serge Hallyn <serge@hallyn.com> > > 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? > > Good questions, and I can brief a bit about the context of the > pre-existing art and this change. > > This complexity was formed in 2014 when I originally developed TPM2 > support and the only available testing plaform was early Intel PTT with > a flakky version of TPM2 support (e.g., no localities). > > Since then we haven't had per leaf-driver divergence. > > Further, I think that this type of layout is actually a better fit if > we ever need to quirks for command durations for a particular device, as > then we can migrate to "copy and patch" semantics i.e., have a copy of > this map in the chip structure. > > As per out-of-tree drivers, it's unfortunate reality of out-of-tree > drivers :-) However, this will definitely add some extra work, when > backporting fixes (not overwhelmingly much). > > BR, Jarkko ^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH v2] tpm: use a map for tpm2_calc_ordinal_duration() 2025-09-19 3:49 ` Serge E. Hallyn 2025-09-19 7:05 ` Jarkko Sakkinen @ 2026-06-08 14:46 ` Benoit HOUYERE 2026-06-09 15:44 ` Jarkko Sakkinen 1 sibling, 1 reply; 7+ messages in thread From: Benoit HOUYERE @ 2026-06-08 14:46 UTC (permalink / raw) To: Serge E. Hallyn, Jarkko Sakkinen Cc: linux-integrity@vger.kernel.org, Frédéric Jouen, Peter Huewe, Jason Gunthorpe, James Bottomley, Mimi Zohar, David Howells, Paul Moore, James Morris, open list, open list:KEYS-TRUSTED, open list:SECURITY SUBSYSTEM, Laurent CHARPENTIER 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 -----Original Message----- From: Serge E. Hallyn <serge@hallyn.com> Sent: Friday, September 19, 2025 5:49 AM To: Jarkko Sakkinen <jarkko@kernel.org> Cc: linux-integrity@vger.kernel.org; Frédéric Jouen <fjouen@sealsq.com>; Peter Huewe <peterhuewe@gmx.de>; Jason Gunthorpe <jgg@ziepe.ca>; James Bottomley <James.Bottomley@hansenpartnership.com>; Mimi Zohar <zohar@linux.ibm.com>; David Howells <dhowells@redhat.com>; Paul Moore <paul@paul-moore.com>; James Morris <jmorris@namei.org>; Serge E. Hallyn <serge@hallyn.com>; open list <linux-kernel@vger.kernel.org>; open list:KEYS-TRUSTED <keyrings@vger.kernel.org>; open list:SECURITY SUBSYSTEM <linux-security-module@vger.kernel.org> 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 <fjouen@sealsq.com> > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org> 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] tpm: use a map for tpm2_calc_ordinal_duration() 2026-06-08 14:46 ` Benoit HOUYERE @ 2026-06-09 15:44 ` Jarkko Sakkinen 0 siblings, 0 replies; 7+ messages in thread From: Jarkko Sakkinen @ 2026-06-09 15:44 UTC (permalink / raw) To: Benoit HOUYERE Cc: Serge E. Hallyn, linux-integrity@vger.kernel.org, Frédéric Jouen, Peter Huewe, Jason Gunthorpe, James Bottomley, Mimi Zohar, David Howells, Paul Moore, James Morris, open list, open list:KEYS-TRUSTED, open list:SECURITY SUBSYSTEM, Laurent CHARPENTIER 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 <serge@hallyn.com> > Sent: Friday, September 19, 2025 5:49 AM > To: Jarkko Sakkinen <jarkko@kernel.org> > Cc: linux-integrity@vger.kernel.org; Frédéric Jouen <fjouen@sealsq.com>; Peter Huewe <peterhuewe@gmx.de>; Jason Gunthorpe <jgg@ziepe.ca>; James Bottomley <James.Bottomley@hansenpartnership.com>; Mimi Zohar <zohar@linux.ibm.com>; David Howells <dhowells@redhat.com>; Paul Moore <paul@paul-moore.com>; James Morris <jmorris@namei.org>; Serge E. Hallyn <serge@hallyn.com>; open list <linux-kernel@vger.kernel.org>; open list:KEYS-TRUSTED <keyrings@vger.kernel.org>; open list:SECURITY SUBSYSTEM <linux-security-module@vger.kernel.org> > 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 <fjouen@sealsq.com> > > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org> > > 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-06-09 15:44 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-09-18 19:30 [PATCH v2] tpm: use a map for tpm2_calc_ordinal_duration() Jarkko Sakkinen 2025-09-18 19:37 ` Jarkko Sakkinen 2025-09-19 3:49 ` Serge E. Hallyn 2025-09-19 7:05 ` Jarkko Sakkinen 2025-09-19 14:47 ` Serge E. Hallyn 2026-06-08 14:46 ` Benoit HOUYERE 2026-06-09 15:44 ` Jarkko Sakkinen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox