All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jarkko Sakkinen <jarkko@kernel.org>
To: Surendran Kanagaraj <surenkj@amazon.com>
Cc: Peter Huewe <peterhuewe@gmx.de>, Jason Gunthorpe <jgg@ziepe.ca>,
	linux-integrity@vger.kernel.org, linux-kernel@vger.kernel.org,
	Breno Leitao <leitao@debian.org>,
	nh-open-source@amazon.com, Alexander Graf <graf@amazon.de>,
	Gunnar Kudrjavets <gunnarku@amazon.com>,
	Josh Levinson <joshlev@amazon.com>
Subject: Re: [PATCH v2 1/2] tpm: Add support for updating TPM2 durations
Date: Sat, 17 Oct 2026 03:19:23 +0300	[thread overview]
Message-ID: <atK_CxpdR-h_DIn6@kernel.org> (raw)
In-Reply-To: <atK9VuMmeD3njBsz@kernel.org>

On Sat, Oct 17, 2026 at 03:12:10AM +0300, Jarkko Sakkinen wrote:
> On Fri, Oct 16, 2026 at 04:37:32AM +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).
> > 
> > Devices that have different timeout requirements than the TPM2 spec
> > could exhaust the retry budget or exceed the command duration. These
> > failures disable the device during an auth session, failing all
> > subsequent TPM requests. Worse, when a TPM2_CC_FLUSH_CONTEXT command
> > fails, it leaks the TPM's transient memory:
> > 
> >     tpm tpm0: in retry loop
> >     tpm tpm0: tpm2_load_context: failed with a TPM error 0x0922
> >     ...
> >     tpm tpm0: A TPM error (2338) occurred flushing context
> > 
> > Fix this by overriding the command durations and retry budget
> > based on chip->duration and letting the driver modify the durations
> > during tpm2_get_timeouts() as TPM 1.x does. Chips without
> > the override keep the current durations.
> > 
> > Tested in QEMU with swtpm on tpm_crb, tpm_tis, tpm_tis_i2c, tpm_ibmvtpm
> > and tpm_vtpm_proxy.
> > 
> > Assisted-by: LLM
> > Signed-off-by: Surendran Kanagaraj <surenkj@amazon.com>
> > ---
> > V1 -> V2:
> > - Drop chip->busy_timeout_ms. TPM2 durations and the TPM2_RC_RETRY
> >   budget now come from chip->duration, which drivers adjust through
> >   ->update_durations() as TPM 1.x does.
> > - Map TPM2_CC_SELF_TEST and TPM2_CC_VERIFY_SIGNATURE to TPM_LONG with
> >   floors of 3s and 30s since their durations do not fit SHORT, MEDIUM,
> >   LONG durations.
> > - Keep the TPM2 defaults for commands sent before tpm2_get_timeouts()
> >   has run such as the one in tpm2_probe() and keep the 2s retry
> >   budget for TPM 1.x chips.
> > - Keep the first TPM2_RC_RETRY wait at TPM2_DURATION_SHORT instead of
> >   chip->duration[TPM_SHORT].
> > - Return early from tpm_tis_update_durations() for TPM2 chips, since
> >   its overrides are for TPM 1.x.
> > - Rename from "tpm: Add per-chip timeout for transient unavailability".
> > 
> > v1: https://lore.kernel.org/linux-integrity/20260923142834.16786-2-surenkj@amazon.com/
> 
> While the issue does get addressed the patch also makes the code
> base worse. What would be more feasible would be to address the
> architectural issue.
> 
> We absolutely do not want a new special to tpm_transmit().

I.e. unifying and adding coherency would be what I would emphasize
here not shortcircuiting the actual issues and adding special
conditions.

I think this change is optimizing for being non-distruptive as an
optimal goals e.g., by not reconsidering TPM ops etc. and like
the bird eye picture. I think this would be a good chance to
do something much more long term than this.

This is BTW something to look up for with LLM generated changes: LLMs
often optimize towards a bug fix type of minimum side-effect change. It
is probably good to make this type of reflection when pre-checking any
feature patch. I'm not complaining, this isjust something that I just
realized. Just another way to look up whether LLM generated result is
"good" or "bad" because it is very easy to get a blind eye for this.

Br, Jarkko

  reply	other threads:[~2026-10-17  0:19 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-16  4:37 [PATCH v2 0/2] tpm: Handle transient NitroTPM unavailability Surendran Kanagaraj
2026-10-16  4:37 ` [PATCH v2 1/2] tpm: Add support for updating TPM2 durations Surendran Kanagaraj
2026-10-17  0:12   ` Jarkko Sakkinen
2026-10-17  0:19     ` Jarkko Sakkinen [this message]
2026-10-16  4:37 ` [PATCH v2 2/2] tpm_crb: Raise durations for Amazon NitroTPM Surendran Kanagaraj
2026-10-16 16:27   ` Jonathan McDowell
2026-10-16 16:38     ` Graf (AWS), Alexander
2026-10-17  0:28       ` Jarkko Sakkinen
2026-10-17  0:31   ` Jarkko Sakkinen
2026-10-17  0:40     ` Jarkko Sakkinen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=atK_CxpdR-h_DIn6@kernel.org \
    --to=jarkko@kernel.org \
    --cc=graf@amazon.de \
    --cc=gunnarku@amazon.com \
    --cc=jgg@ziepe.ca \
    --cc=joshlev@amazon.com \
    --cc=leitao@debian.org \
    --cc=linux-integrity@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nh-open-source@amazon.com \
    --cc=peterhuewe@gmx.de \
    --cc=surenkj@amazon.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.