* [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; 5+ 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] 5+ 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; 5+ 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] 5+ 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
1 sibling, 1 reply; 5+ 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] 5+ 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
0 siblings, 1 reply; 5+ 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] 5+ 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; 5+ 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] 5+ messages in thread
end of thread, other threads:[~2025-09-19 14:48 UTC | newest]
Thread overview: 5+ 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
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).