Re: [PATCH v2 2/2] tpm_crb: Raise durations for Amazon NitroTPM

[Jarkko Sakkinen <jarkko@kernel.org>]

On Fri, Oct 16, 2026 at 04:37:33AM +0000, Surendran Kanagaraj wrote:
> TPM commands need to complete within the command duration defined in
> the TPM2 spec or keep answering TPM2_RC_RETRY for at most
> TPM2_DURATION_LONG (2s).
> 
> In rare scenarios NitroTPM can enter a mode where it either sends
> TPM2_RC_RETRY for more than 2s or delays command completion. When
> this happens during auth session start, the driver disables the chip
> and every subsequent request fails:
> 
>     tpm tpm0: in retry loop
>     tpm tpm0: tpm2_load_context: failed with a TPM error 0x0922
> 
> Add support for vendor duration overrides to the CRB driver, based on
> the vendor ID read from the interface ID register. For NitroTPM, raise
> TPM_SHORT, TPM_MEDIUM and TPM_LONG to 30s which covers the longest
> unavailability time along with some headroom.
> 
> Tested on an EC2 instance with NitroTPM by inducing multiple
> unavailability windows with no TPM errors, and in QEMU with swtpm by
> holding the TPM in TPM2_RC_RETRY and stalling command completion for
> longer than 2s.
> 
> Assisted-by: LLM
> Signed-off-by: Surendran Kanagaraj <surenkj@amazon.com>
> ---
> V1 -> V2:
> - Raise the durations through ->update_durations() instead of setting
>   chip->busy_timeout_ms.
> - Raise TPM_SHORT, TPM_MEDIUM and TPM_LONG to 30s.
> - Replace the vendor quirk table with a plain vendor ID check.
> - Use the TPM_CRB_ prefix for the new macros.
> - Rename from "tpm_crb: Raise timeouts for Amazon NitroTPM".
> 
> v1: https://lore.kernel.org/linux-integrity/20260923142834.16786-3-surenkj@amazon.com/
> ---
>  drivers/char/tpm/tpm_crb.c | 44 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 44 insertions(+)
> 
> diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
> index ceb4100ba400..6ba9016d6463 100644
> --- a/drivers/char/tpm/tpm_crb.c
> +++ b/drivers/char/tpm/tpm_crb.c
> @@ -13,6 +13,7 @@
>  
>  #include <linux/acpi.h>
>  #include <linux/highmem.h>
> +#include <linux/io-64-nonatomic-lo-hi.h>
>  #include <linux/rculist.h>
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
> @@ -26,6 +27,15 @@
>  #define ACPI_SIG_TPM2 "TPM2"
>  #define TPM_CRB_MAX_RESOURCES 3
>  
> +/* TPM_CRB_INTF_ID_x vendor ID field, bits 47:32 of the 64-bit register */
> +#define TPM_CRB_INTF_ID_VID(intf_id)	(((intf_id) >> 32) & 0xffff)
> +
> +/* Amazon NitroTPM */
> +#define TPM_CRB_INTF_VID_AMZN		0x0ec2
> +
> +/* Longest NitroTPM unavailability, with some headroom */
> +#define TPM_CRB_AMZN_DURATION_MS	30000
> +
>  static const guid_t crb_acpi_start_guid =
>  	GUID_INIT(0x6BBF6CAB, 0x5463, 0x4714,
>  		  0xB7, 0xCD, 0xF0, 0x20, 0x3C, 0x03, 0x68, 0xD4);
> @@ -521,6 +531,39 @@ static bool crb_req_canceled(struct tpm_chip *chip, u8 status)
>  	return (cancel & CRB_CANCEL_INVOKE) == CRB_CANCEL_INVOKE;
>  }
>  
> +/*
> + * Read the vendor ID from the interface ID register. Returns -ENODEV when
> + * the head registers are not mapped for this start method.
> + */
> +static int crb_vendor_id(struct crb_priv *priv)
> +{
> +	u64 intf_id;
> +
> +	if (!priv->regs_h)
> +		return -ENODEV;
> +
> +	intf_id = lo_hi_readq(&priv->regs_h->intf_id);
> +
> +	return TPM_CRB_INTF_ID_VID(intf_id);
> +}
> +
> +static void crb_update_durations(struct tpm_chip *chip,
> +				 unsigned long *duration_cap)
> +{
> +	struct crb_priv *priv = dev_get_drvdata(&chip->dev);
> +	unsigned long duration;
> +
> +	chip->duration_adjusted = false;
> +
> +	if (crb_vendor_id(priv) == TPM_CRB_INTF_VID_AMZN) {
> +		duration = msecs_to_jiffies(TPM_CRB_AMZN_DURATION_MS);
> +		duration_cap[TPM_SHORT] = duration;
> +		duration_cap[TPM_MEDIUM] = duration;
> +		duration_cap[TPM_LONG] = duration;
> +		chip->duration_adjusted = true;
> +	}
> +}
> +
>  static const struct tpm_class_ops tpm_crb = {
>  	.flags = TPM_OPS_AUTO_STARTUP,
>  	.status = crb_status,
> @@ -532,6 +575,7 @@ static const struct tpm_class_ops tpm_crb = {
>  	.cmd_ready = crb_cmd_ready,
>  	.request_locality = crb_request_locality,
>  	.relinquish_locality = crb_relinquish_locality,
> +	.update_durations = crb_update_durations,
>  	.req_complete_mask = CRB_DRV_STS_COMPLETE,
>  	.req_complete_val = CRB_DRV_STS_COMPLETE,
>  };
> -- 
> 2.47.3
> 

Yeah so this needs a bit more redesign and while it is a possible
way to address all these issues I don't believe it is best possible
way imaginable to do something that lasts long-term.

I don't want to go into concrete proposals because then I would have
to really engineer the whole change while at it because I would
likely try a number of approaches out. I.e. just a bit more effort
and innovation please.

Br, Jarkko