mirror of https://lore.kernel.org/lkml/
 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®