* [PATCH] tpm: tis: Increase the default for timeouts B and C
@ 2025-04-02 17:21 Michal Suchanek
2025-04-02 17:45 ` Jonathan McDowell
2025-04-03 18:38 ` Jarkko Sakkinen
0 siblings, 2 replies; 26+ messages in thread
From: Michal Suchanek @ 2025-04-02 17:21 UTC (permalink / raw)
Cc: Michal Suchanek, Peter Huewe, Jarkko Sakkinen, Jason Gunthorpe,
linux-integrity, linux-kernel, Jonathan McDowell
With some Infineon chips the timeouts in tpm_tis_send_data (both B and
C) can reach up to about 2250 ms.
Extend the timeout duration to accommodate this.
Link: https://lore.kernel.org/linux-integrity/Z5pI07m0Muapyu9w@kitsune.suse.cz/
Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
An alternative would be to add an entry to vendor_timeout_overrides but
I do not know how to determine the chip IDs to put into this table.
---
drivers/char/tpm/tpm_tis_core.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h
index 970d02c337c7..1ff565be2175 100644
--- a/drivers/char/tpm/tpm_tis_core.h
+++ b/drivers/char/tpm/tpm_tis_core.h
@@ -54,7 +54,7 @@ enum tis_int_flags {
enum tis_defaults {
TIS_MEM_LEN = 0x5000,
TIS_SHORT_TIMEOUT = 750, /* ms */
- TIS_LONG_TIMEOUT = 2000, /* 2 sec */
+ TIS_LONG_TIMEOUT = 4000, /* 2 sec */
TIS_TIMEOUT_MIN_ATML = 14700, /* usecs */
TIS_TIMEOUT_MAX_ATML = 15000, /* usecs */
};
@@ -64,7 +64,7 @@ enum tis_defaults {
*/
#define TIS_TIMEOUT_A_MAX max_t(int, TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_A)
#define TIS_TIMEOUT_B_MAX max_t(int, TIS_LONG_TIMEOUT, TPM2_TIMEOUT_B)
-#define TIS_TIMEOUT_C_MAX max_t(int, TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_C)
+#define TIS_TIMEOUT_C_MAX max_t(int, TIS_LONG_TIMEOUT, TPM2_TIMEOUT_C)
#define TIS_TIMEOUT_D_MAX max_t(int, TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_D)
#define TPM_ACCESS(l) (0x0000 | ((l) << 12))
--
2.47.1
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH] tpm: tis: Increase the default for timeouts B and C
2025-04-02 17:21 [PATCH] tpm: tis: Increase the default for timeouts B and C Michal Suchanek
@ 2025-04-02 17:45 ` Jonathan McDowell
2025-04-02 20:07 ` Michal Suchánek
2025-04-03 18:45 ` [PATCH] tpm: tis: Increase the default for timeouts B and C Jarkko Sakkinen
2025-04-03 18:38 ` Jarkko Sakkinen
1 sibling, 2 replies; 26+ messages in thread
From: Jonathan McDowell @ 2025-04-02 17:45 UTC (permalink / raw)
To: Michal Suchanek
Cc: Peter Huewe, Jarkko Sakkinen, Jason Gunthorpe, linux-integrity,
linux-kernel
On Wed, Apr 02, 2025 at 07:21:30PM +0200, Michal Suchanek wrote:
>With some Infineon chips the timeouts in tpm_tis_send_data (both B and
>C) can reach up to about 2250 ms.
>
>Extend the timeout duration to accommodate this.
The problem here is the bump of timeout_c is going to interact poorly
with the Infineon errata workaround, as now we'll wait 4s instead of
200ms to detect the stuck status change.
(Also shouldn't timeout_c already end up as 750ms, as it's
max(TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_C), and TIS_SHORT_TIMEOUT is 750 vs
200 for TPM2_TIMEOUT_C? That doesn't seem to be borne out by your logs,
nor my results.)
>Link: https://lore.kernel.org/linux-integrity/Z5pI07m0Muapyu9w@kitsune.suse.cz/
>Signed-off-by: Michal Suchanek <msuchanek@suse.de>
>---
>An alternative would be to add an entry to vendor_timeout_overrides but
>I do not know how to determine the chip IDs to put into this table.
>---
> drivers/char/tpm/tpm_tis_core.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h
>index 970d02c337c7..1ff565be2175 100644
>--- a/drivers/char/tpm/tpm_tis_core.h
>+++ b/drivers/char/tpm/tpm_tis_core.h
>@@ -54,7 +54,7 @@ enum tis_int_flags {
> enum tis_defaults {
> TIS_MEM_LEN = 0x5000,
> TIS_SHORT_TIMEOUT = 750, /* ms */
>- TIS_LONG_TIMEOUT = 2000, /* 2 sec */
>+ TIS_LONG_TIMEOUT = 4000, /* 2 sec */
> TIS_TIMEOUT_MIN_ATML = 14700, /* usecs */
> TIS_TIMEOUT_MAX_ATML = 15000, /* usecs */
> };
>@@ -64,7 +64,7 @@ enum tis_defaults {
> */
> #define TIS_TIMEOUT_A_MAX max_t(int, TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_A)
> #define TIS_TIMEOUT_B_MAX max_t(int, TIS_LONG_TIMEOUT, TPM2_TIMEOUT_B)
>-#define TIS_TIMEOUT_C_MAX max_t(int, TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_C)
>+#define TIS_TIMEOUT_C_MAX max_t(int, TIS_LONG_TIMEOUT, TPM2_TIMEOUT_C)
> #define TIS_TIMEOUT_D_MAX max_t(int, TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_D)
>
> #define TPM_ACCESS(l) (0x0000 | ((l) << 12))
>--
>2.47.1
>
J.
--
... "Tom's root boot is the Linux world equivalent of a 'get out of jail
free' card. The man is a *hero*." -- Simon Brooke, ucol.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] tpm: tis: Increase the default for timeouts B and C
2025-04-02 17:45 ` Jonathan McDowell
@ 2025-04-02 20:07 ` Michal Suchánek
2025-04-03 9:31 ` Michal Suchánek
2025-04-03 11:00 ` Jonathan McDowell
2025-04-03 18:45 ` [PATCH] tpm: tis: Increase the default for timeouts B and C Jarkko Sakkinen
1 sibling, 2 replies; 26+ messages in thread
From: Michal Suchánek @ 2025-04-02 20:07 UTC (permalink / raw)
To: Jonathan McDowell
Cc: Peter Huewe, Jarkko Sakkinen, Jason Gunthorpe, linux-integrity,
linux-kernel
On Wed, Apr 02, 2025 at 06:45:40PM +0100, Jonathan McDowell wrote:
> On Wed, Apr 02, 2025 at 07:21:30PM +0200, Michal Suchanek wrote:
> > With some Infineon chips the timeouts in tpm_tis_send_data (both B and
> > C) can reach up to about 2250 ms.
> >
> > Extend the timeout duration to accommodate this.
>
> The problem here is the bump of timeout_c is going to interact poorly with
> the Infineon errata workaround, as now we'll wait 4s instead of 200ms to
> detect the stuck status change.
Yes, that's problematic. Is it possible to detect the errata by anything
other than waiting for the timeout to expire?
>
> (Also shouldn't timeout_c already end up as 750ms, as it's
> max(TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_C), and TIS_SHORT_TIMEOUT is 750 vs 200
> for TPM2_TIMEOUT_C? That doesn't seem to be borne out by your logs, nor my
> results.)
Indeed, it should be 750ms but the logs show 200ms. I do not see
where it could get reduced, nor any significan difference between the
mainline code and the kernel I am using in this area.
Thanks
Michal
>
> > Link: https://lore.kernel.org/linux-integrity/Z5pI07m0Muapyu9w@kitsune.suse.cz/
> > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > ---
> > An alternative would be to add an entry to vendor_timeout_overrides but
> > I do not know how to determine the chip IDs to put into this table.
> > ---
> > drivers/char/tpm/tpm_tis_core.h | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h
> > index 970d02c337c7..1ff565be2175 100644
> > --- a/drivers/char/tpm/tpm_tis_core.h
> > +++ b/drivers/char/tpm/tpm_tis_core.h
> > @@ -54,7 +54,7 @@ enum tis_int_flags {
> > enum tis_defaults {
> > TIS_MEM_LEN = 0x5000,
> > TIS_SHORT_TIMEOUT = 750, /* ms */
> > - TIS_LONG_TIMEOUT = 2000, /* 2 sec */
> > + TIS_LONG_TIMEOUT = 4000, /* 2 sec */
> > TIS_TIMEOUT_MIN_ATML = 14700, /* usecs */
> > TIS_TIMEOUT_MAX_ATML = 15000, /* usecs */
> > };
> > @@ -64,7 +64,7 @@ enum tis_defaults {
> > */
> > #define TIS_TIMEOUT_A_MAX max_t(int, TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_A)
> > #define TIS_TIMEOUT_B_MAX max_t(int, TIS_LONG_TIMEOUT, TPM2_TIMEOUT_B)
> > -#define TIS_TIMEOUT_C_MAX max_t(int, TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_C)
> > +#define TIS_TIMEOUT_C_MAX max_t(int, TIS_LONG_TIMEOUT, TPM2_TIMEOUT_C)
> > #define TIS_TIMEOUT_D_MAX max_t(int, TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_D)
> >
> > #define TPM_ACCESS(l) (0x0000 | ((l) << 12))
> > --
> > 2.47.1
> >
>
> J.
>
> --
> ... "Tom's root boot is the Linux world equivalent of a 'get out of jail
> free' card. The man is a *hero*." -- Simon Brooke, ucol.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] tpm: tis: Increase the default for timeouts B and C
2025-04-02 20:07 ` Michal Suchánek
@ 2025-04-03 9:31 ` Michal Suchánek
2025-04-03 11:00 ` Jonathan McDowell
1 sibling, 0 replies; 26+ messages in thread
From: Michal Suchánek @ 2025-04-03 9:31 UTC (permalink / raw)
To: Jonathan McDowell
Cc: Peter Huewe, Jarkko Sakkinen, Jason Gunthorpe, linux-integrity,
linux-kernel
On Wed, Apr 02, 2025 at 10:07:40PM +0200, Michal Suchánek wrote:
> On Wed, Apr 02, 2025 at 06:45:40PM +0100, Jonathan McDowell wrote:
> > On Wed, Apr 02, 2025 at 07:21:30PM +0200, Michal Suchanek wrote:
> > > With some Infineon chips the timeouts in tpm_tis_send_data (both B and
> > > C) can reach up to about 2250 ms.
> > >
> > > Extend the timeout duration to accommodate this.
> >
> > The problem here is the bump of timeout_c is going to interact poorly with
> > the Infineon errata workaround, as now we'll wait 4s instead of 200ms to
> > detect the stuck status change.
>
> Yes, that's problematic. Is it possible to detect the errata by anything
> other than waiting for the timeout to expire?
>
> >
> > (Also shouldn't timeout_c already end up as 750ms, as it's
> > max(TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_C), and TIS_SHORT_TIMEOUT is 750 vs 200
> > for TPM2_TIMEOUT_C? That doesn't seem to be borne out by your logs, nor my
> > results.)
>
> Indeed, it should be 750ms but the logs show 200ms. I do not see
> where it could get reduced, nor any significan difference between the
> mainline code and the kernel I am using in this area.
This would come from
drivers/char/tpm/tpm2-cmd.c: chip->timeout_c = msecs_to_jiffies(TPM2_TIMEOUT_C);
include/linux/tpm.h: TPM2_TIMEOUT_C = 200,
So this would need also adjusting.
Thanks
Michal
>
> Thanks
>
> Michal
>
> >
> > > Link: https://lore.kernel.org/linux-integrity/Z5pI07m0Muapyu9w@kitsune.suse.cz/
> > > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > > ---
> > > An alternative would be to add an entry to vendor_timeout_overrides but
> > > I do not know how to determine the chip IDs to put into this table.
> > > ---
> > > drivers/char/tpm/tpm_tis_core.h | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h
> > > index 970d02c337c7..1ff565be2175 100644
> > > --- a/drivers/char/tpm/tpm_tis_core.h
> > > +++ b/drivers/char/tpm/tpm_tis_core.h
> > > @@ -54,7 +54,7 @@ enum tis_int_flags {
> > > enum tis_defaults {
> > > TIS_MEM_LEN = 0x5000,
> > > TIS_SHORT_TIMEOUT = 750, /* ms */
> > > - TIS_LONG_TIMEOUT = 2000, /* 2 sec */
> > > + TIS_LONG_TIMEOUT = 4000, /* 2 sec */
> > > TIS_TIMEOUT_MIN_ATML = 14700, /* usecs */
> > > TIS_TIMEOUT_MAX_ATML = 15000, /* usecs */
> > > };
> > > @@ -64,7 +64,7 @@ enum tis_defaults {
> > > */
> > > #define TIS_TIMEOUT_A_MAX max_t(int, TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_A)
> > > #define TIS_TIMEOUT_B_MAX max_t(int, TIS_LONG_TIMEOUT, TPM2_TIMEOUT_B)
> > > -#define TIS_TIMEOUT_C_MAX max_t(int, TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_C)
> > > +#define TIS_TIMEOUT_C_MAX max_t(int, TIS_LONG_TIMEOUT, TPM2_TIMEOUT_C)
> > > #define TIS_TIMEOUT_D_MAX max_t(int, TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_D)
> > >
> > > #define TPM_ACCESS(l) (0x0000 | ((l) << 12))
> > > --
> > > 2.47.1
> > >
> >
> > J.
> >
> > --
> > ... "Tom's root boot is the Linux world equivalent of a 'get out of jail
> > free' card. The man is a *hero*." -- Simon Brooke, ucol.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] tpm: tis: Increase the default for timeouts B and C
2025-04-02 20:07 ` Michal Suchánek
2025-04-03 9:31 ` Michal Suchánek
@ 2025-04-03 11:00 ` Jonathan McDowell
2025-04-03 11:56 ` Michal Suchánek
1 sibling, 1 reply; 26+ messages in thread
From: Jonathan McDowell @ 2025-04-03 11:00 UTC (permalink / raw)
To: Michal Suchánek
Cc: Peter Huewe, Jarkko Sakkinen, Jason Gunthorpe, linux-integrity,
linux-kernel
On Wed, Apr 02, 2025 at 10:07:39PM +0200, Michal Suchánek wrote:
>On Wed, Apr 02, 2025 at 06:45:40PM +0100, Jonathan McDowell wrote:
>> On Wed, Apr 02, 2025 at 07:21:30PM +0200, Michal Suchanek wrote:
>> > With some Infineon chips the timeouts in tpm_tis_send_data (both B and
>> > C) can reach up to about 2250 ms.
>> >
>> > Extend the timeout duration to accommodate this.
>>
>> The problem here is the bump of timeout_c is going to interact poorly with
>> the Infineon errata workaround, as now we'll wait 4s instead of 200ms to
>> detect the stuck status change.
>
>Yes, that's problematic. Is it possible to detect the errata by anything
>other than waiting for the timeout to expire?
Not that I'm aware of, nor have seen in my experimentation. It's a
"stuck" status, so the timeout is how it's detected.
OOI, have you tried back porting the fixes that are in mainline for 6.15
to your frankenkernel? I _think_ the errata fix might end up resolving
at least the timeout for valid for you, as a side effect? We're
currently rolling them out across our fleet, but I don't have enough
runtime yet to be sure they've sorted all the timeout instances we see.
J.
--
/-\ | He's weird? It's ok, I'm fluent in
|@/ Debian GNU/Linux Developer | weird.
\- |
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] tpm: tis: Increase the default for timeouts B and C
2025-04-03 11:00 ` Jonathan McDowell
@ 2025-04-03 11:56 ` Michal Suchánek
2025-04-03 13:00 ` Jonathan McDowell
0 siblings, 1 reply; 26+ messages in thread
From: Michal Suchánek @ 2025-04-03 11:56 UTC (permalink / raw)
To: Jonathan McDowell
Cc: Peter Huewe, Jarkko Sakkinen, Jason Gunthorpe, linux-integrity,
linux-kernel
On Thu, Apr 03, 2025 at 12:00:36PM +0100, Jonathan McDowell wrote:
> On Wed, Apr 02, 2025 at 10:07:39PM +0200, Michal Suchánek wrote:
> > On Wed, Apr 02, 2025 at 06:45:40PM +0100, Jonathan McDowell wrote:
> > > On Wed, Apr 02, 2025 at 07:21:30PM +0200, Michal Suchanek wrote:
> > > > With some Infineon chips the timeouts in tpm_tis_send_data (both B and
> > > > C) can reach up to about 2250 ms.
> > > >
> > > > Extend the timeout duration to accommodate this.
> > >
> > > The problem here is the bump of timeout_c is going to interact poorly with
> > > the Infineon errata workaround, as now we'll wait 4s instead of 200ms to
> > > detect the stuck status change.
> >
> > Yes, that's problematic. Is it possible to detect the errata by anything
> > other than waiting for the timeout to expire?
>
> Not that I'm aware of, nor have seen in my experimentation. It's a "stuck"
> status, so the timeout is how it's detected.
>
> OOI, have you tried back porting the fixes that are in mainline for 6.15 to
> your frankenkernel? I _think_ the errata fix might end up resolving at least
> the timeout for valid for you, as a side effect? We're currently rolling
> them out across our fleet, but I don't have enough runtime yet to be sure
> they've sorted all the timeout instances we see.
When was that merged?
The change I see is that sometimes EAGAIN is returned instead of ETIME
but based on the previous discussion this is unlikely to help.
Thanks
Michal
>
> J.
>
> --
> /-\ | He's weird? It's ok, I'm fluent in
> |@/ Debian GNU/Linux Developer | weird.
> \- |
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] tpm: tis: Increase the default for timeouts B and C
2025-04-03 11:56 ` Michal Suchánek
@ 2025-04-03 13:00 ` Jonathan McDowell
2025-04-03 14:11 ` Michal Suchánek
2025-04-03 18:25 ` [PATCH] tpm: tis: Increase the default for timeout B Michal Suchanek
0 siblings, 2 replies; 26+ messages in thread
From: Jonathan McDowell @ 2025-04-03 13:00 UTC (permalink / raw)
To: Michal Suchánek
Cc: Peter Huewe, Jarkko Sakkinen, Jason Gunthorpe, linux-integrity,
linux-kernel
On Thu, Apr 03, 2025 at 01:56:37PM +0200, Michal Suchánek wrote:
>On Thu, Apr 03, 2025 at 12:00:36PM +0100, Jonathan McDowell wrote:
>> On Wed, Apr 02, 2025 at 10:07:39PM +0200, Michal Suchánek wrote:
>> > On Wed, Apr 02, 2025 at 06:45:40PM +0100, Jonathan McDowell wrote:
>> > > On Wed, Apr 02, 2025 at 07:21:30PM +0200, Michal Suchanek wrote:
>> > > > With some Infineon chips the timeouts in tpm_tis_send_data (both B and
>> > > > C) can reach up to about 2250 ms.
>> > > >
>> > > > Extend the timeout duration to accommodate this.
>> > >
>> > > The problem here is the bump of timeout_c is going to interact poorly with
>> > > the Infineon errata workaround, as now we'll wait 4s instead of 200ms to
>> > > detect the stuck status change.
>> >
>> > Yes, that's problematic. Is it possible to detect the errata by anything
>> > other than waiting for the timeout to expire?
>>
>> Not that I'm aware of, nor have seen in my experimentation. It's a "stuck"
>> status, so the timeout is how it's detected.
>>
>> OOI, have you tried back porting the fixes that are in mainline for 6.15 to
>> your frankenkernel? I _think_ the errata fix might end up resolving at least
>> the timeout for valid for you, as a side effect? We're currently rolling
>> them out across our fleet, but I don't have enough runtime yet to be sure
>> they've sorted all the timeout instances we see.
>
>When was that merged?
It hit Linus' tree last Friday I believe.
>The change I see is that sometimes EAGAIN is returned instead of ETIME
>but based on the previous discussion this is unlikely to help.
That sounds like you might have picked up the version with the typo that
I posted to the list; it got fixed up before making it to mainline. The
two patches I've backported locally are in mainline as:
7146dffa875cd00e7a7f918e1fce79c7593ac1fa tpm, tpm_tis: Fix timeout handling when waiting for TPM status
de9e33df7762abbfc2a1568291f2c3a3154c6a9d tpm, tpm_tis: Workaround failed command reception on Infineon devices
J.
--
Can I trade this job for what's behind door 2?
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] tpm: tis: Increase the default for timeouts B and C
2025-04-03 13:00 ` Jonathan McDowell
@ 2025-04-03 14:11 ` Michal Suchánek
2025-04-03 18:25 ` [PATCH] tpm: tis: Increase the default for timeout B Michal Suchanek
1 sibling, 0 replies; 26+ messages in thread
From: Michal Suchánek @ 2025-04-03 14:11 UTC (permalink / raw)
To: Jonathan McDowell
Cc: Peter Huewe, Jarkko Sakkinen, Jason Gunthorpe, linux-integrity,
linux-kernel
On Thu, Apr 03, 2025 at 02:00:26PM +0100, Jonathan McDowell wrote:
> On Thu, Apr 03, 2025 at 01:56:37PM +0200, Michal Suchánek wrote:
> > On Thu, Apr 03, 2025 at 12:00:36PM +0100, Jonathan McDowell wrote:
> > > On Wed, Apr 02, 2025 at 10:07:39PM +0200, Michal Suchánek wrote:
> > > > On Wed, Apr 02, 2025 at 06:45:40PM +0100, Jonathan McDowell wrote:
> > > > > On Wed, Apr 02, 2025 at 07:21:30PM +0200, Michal Suchanek wrote:
> > > > > > With some Infineon chips the timeouts in tpm_tis_send_data (both B and
> > > > > > C) can reach up to about 2250 ms.
> > > > > >
> > > > > > Extend the timeout duration to accommodate this.
> > > > >
> > > > > The problem here is the bump of timeout_c is going to interact poorly with
> > > > > the Infineon errata workaround, as now we'll wait 4s instead of 200ms to
> > > > > detect the stuck status change.
> > > >
> > > > Yes, that's problematic. Is it possible to detect the errata by anything
> > > > other than waiting for the timeout to expire?
> > >
> > > Not that I'm aware of, nor have seen in my experimentation. It's a "stuck"
> > > status, so the timeout is how it's detected.
> > >
> > > OOI, have you tried back porting the fixes that are in mainline for 6.15 to
> > > your frankenkernel? I _think_ the errata fix might end up resolving at least
> > > the timeout for valid for you, as a side effect? We're currently rolling
> > > them out across our fleet, but I don't have enough runtime yet to be sure
> > > they've sorted all the timeout instances we see.
> >
> > When was that merged?
>
> It hit Linus' tree last Friday I believe.
>
> > The change I see is that sometimes EAGAIN is returned instead of ETIME
> > but based on the previous discussion this is unlikely to help.
>
> That sounds like you might have picked up the version with the typo that I
> posted to the list; it got fixed up before making it to mainline. The two
> patches I've backported locally are in mainline as:
>
> 7146dffa875cd00e7a7f918e1fce79c7593ac1fa tpm, tpm_tis: Fix timeout handling when waiting for TPM status
> de9e33df7762abbfc2a1568291f2c3a3154c6a9d tpm, tpm_tis: Workaround failed command reception on Infineon devices
Indeed, it adds a retry in tpm_send_main as well. That might work, needs
some testing on the affected hardware. With that changing only the B
timeout should suffice.
Thanks
Michal
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH] tpm: tis: Increase the default for timeout B
2025-04-03 13:00 ` Jonathan McDowell
2025-04-03 14:11 ` Michal Suchánek
@ 2025-04-03 18:25 ` Michal Suchanek
2025-04-03 18:28 ` Paul Menzel
2025-04-03 18:49 ` [PATCH] tpm: tis: Increase the default for timeout B Jarkko Sakkinen
1 sibling, 2 replies; 26+ messages in thread
From: Michal Suchanek @ 2025-04-03 18:25 UTC (permalink / raw)
Cc: Michal Suchanek, Peter Huewe, Jarkko Sakkinen, Jason Gunthorpe,
linux-integrity, linux-kernel, Jonathan McDowell
With some Infineon chips the timeouts in tpm_tis_send_data (both B and
C) can reach up to about 2250 ms.
Timeout C is retried since
commit de9e33df7762 ("tpm, tpm_tis: Workaround failed command reception on Infineon devices")
Timeout B still needs to be extended.
Link: https://lore.kernel.org/linux-integrity/Z5pI07m0Muapyu9w@kitsune.suse.cz/
Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
V2: Only extend timeout B
---
drivers/char/tpm/tpm_tis_core.h | 2 +-
include/linux/tpm.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h
index 970d02c337c7..c272c25eb9d4 100644
--- a/drivers/char/tpm/tpm_tis_core.h
+++ b/drivers/char/tpm/tpm_tis_core.h
@@ -54,7 +54,7 @@ enum tis_int_flags {
enum tis_defaults {
TIS_MEM_LEN = 0x5000,
TIS_SHORT_TIMEOUT = 750, /* ms */
- TIS_LONG_TIMEOUT = 2000, /* 2 sec */
+ TIS_LONG_TIMEOUT = 4000, /* 4 sec */
TIS_TIMEOUT_MIN_ATML = 14700, /* usecs */
TIS_TIMEOUT_MAX_ATML = 15000, /* usecs */
};
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index 6c3125300c00..3db0b6a87d45 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -224,7 +224,7 @@ enum tpm2_const {
enum tpm2_timeouts {
TPM2_TIMEOUT_A = 750,
- TPM2_TIMEOUT_B = 2000,
+ TPM2_TIMEOUT_B = 4000,
TPM2_TIMEOUT_C = 200,
TPM2_TIMEOUT_D = 30,
TPM2_DURATION_SHORT = 20,
--
2.47.1
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH] tpm: tis: Increase the default for timeout B
2025-04-03 18:25 ` [PATCH] tpm: tis: Increase the default for timeout B Michal Suchanek
@ 2025-04-03 18:28 ` Paul Menzel
2025-04-04 8:23 ` [PATCH v3] tpm: tis: Double the timeout B to 4s Michal Suchanek
2025-04-03 18:49 ` [PATCH] tpm: tis: Increase the default for timeout B Jarkko Sakkinen
1 sibling, 1 reply; 26+ messages in thread
From: Paul Menzel @ 2025-04-03 18:28 UTC (permalink / raw)
To: Michal Suchanek
Cc: Peter Huewe, Jarkko Sakkinen, Jason Gunthorpe, linux-integrity,
linux-kernel, Jonathan McDowell
Dear Michal,
Thank you for the patch. For the summary/title you could be more
specific by using *Double*:
tpm: tis: Double default for timeout B to 4 s
Am 03.04.25 um 20:25 schrieb Michal Suchanek:
> With some Infineon chips the timeouts in tpm_tis_send_data (both B and
> C) can reach up to about 2250 ms.
>
> Timeout C is retried since
> commit de9e33df7762 ("tpm, tpm_tis: Workaround failed command reception on Infineon devices")
>
> Timeout B still needs to be extended.
It’d be great if you could amend the commit message and add the Infinion
device you have problems with, and maybe also add the error behavior.
> Link: https://lore.kernel.org/linux-integrity/Z5pI07m0Muapyu9w@kitsune.suse.cz/
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---
> V2: Only extend timeout B
> ---
> drivers/char/tpm/tpm_tis_core.h | 2 +-
> include/linux/tpm.h | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h
> index 970d02c337c7..c272c25eb9d4 100644
> --- a/drivers/char/tpm/tpm_tis_core.h
> +++ b/drivers/char/tpm/tpm_tis_core.h
> @@ -54,7 +54,7 @@ enum tis_int_flags {
> enum tis_defaults {
> TIS_MEM_LEN = 0x5000,
> TIS_SHORT_TIMEOUT = 750, /* ms */
> - TIS_LONG_TIMEOUT = 2000, /* 2 sec */
> + TIS_LONG_TIMEOUT = 4000, /* 4 sec */
> TIS_TIMEOUT_MIN_ATML = 14700, /* usecs */
> TIS_TIMEOUT_MAX_ATML = 15000, /* usecs */
> };
> diff --git a/include/linux/tpm.h b/include/linux/tpm.h
> index 6c3125300c00..3db0b6a87d45 100644
> --- a/include/linux/tpm.h
> +++ b/include/linux/tpm.h
> @@ -224,7 +224,7 @@ enum tpm2_const {
>
> enum tpm2_timeouts {
> TPM2_TIMEOUT_A = 750,
> - TPM2_TIMEOUT_B = 2000,
> + TPM2_TIMEOUT_B = 4000,
> TPM2_TIMEOUT_C = 200,
> TPM2_TIMEOUT_D = 30,
> TPM2_DURATION_SHORT = 20,
Kind regards,
Paul
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] tpm: tis: Increase the default for timeouts B and C
2025-04-02 17:21 [PATCH] tpm: tis: Increase the default for timeouts B and C Michal Suchanek
2025-04-02 17:45 ` Jonathan McDowell
@ 2025-04-03 18:38 ` Jarkko Sakkinen
1 sibling, 0 replies; 26+ messages in thread
From: Jarkko Sakkinen @ 2025-04-03 18:38 UTC (permalink / raw)
To: Michal Suchanek
Cc: Peter Huewe, Jason Gunthorpe, linux-integrity, linux-kernel,
Jonathan McDowell
On Wed, Apr 02, 2025 at 07:21:30PM +0200, Michal Suchanek wrote:
> With some Infineon chips the timeouts in tpm_tis_send_data (both B and
> C) can reach up to about 2250 ms.
>
> Extend the timeout duration to accommodate this.
>
> Link: https://lore.kernel.org/linux-integrity/Z5pI07m0Muapyu9w@kitsune.suse.cz/
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---
> An alternative would be to add an entry to vendor_timeout_overrides but
> I do not know how to determine the chip IDs to put into this table.
> ---
> drivers/char/tpm/tpm_tis_core.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h
> index 970d02c337c7..1ff565be2175 100644
> --- a/drivers/char/tpm/tpm_tis_core.h
> +++ b/drivers/char/tpm/tpm_tis_core.h
> @@ -54,7 +54,7 @@ enum tis_int_flags {
> enum tis_defaults {
> TIS_MEM_LEN = 0x5000,
> TIS_SHORT_TIMEOUT = 750, /* ms */
> - TIS_LONG_TIMEOUT = 2000, /* 2 sec */
> + TIS_LONG_TIMEOUT = 4000, /* 2 sec */
/* 4 secs */
> TIS_TIMEOUT_MIN_ATML = 14700, /* usecs */
> TIS_TIMEOUT_MAX_ATML = 15000, /* usecs */
> };
> @@ -64,7 +64,7 @@ enum tis_defaults {
> */
> #define TIS_TIMEOUT_A_MAX max_t(int, TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_A)
> #define TIS_TIMEOUT_B_MAX max_t(int, TIS_LONG_TIMEOUT, TPM2_TIMEOUT_B)
> -#define TIS_TIMEOUT_C_MAX max_t(int, TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_C)
> +#define TIS_TIMEOUT_C_MAX max_t(int, TIS_LONG_TIMEOUT, TPM2_TIMEOUT_C)
> #define TIS_TIMEOUT_D_MAX max_t(int, TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_D)
>
> #define TPM_ACCESS(l) (0x0000 | ((l) << 12))
> --
> 2.47.1
>
>
BR, Jarkko
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] tpm: tis: Increase the default for timeouts B and C
2025-04-02 17:45 ` Jonathan McDowell
2025-04-02 20:07 ` Michal Suchánek
@ 2025-04-03 18:45 ` Jarkko Sakkinen
2025-04-03 20:43 ` Jonathan McDowell
1 sibling, 1 reply; 26+ messages in thread
From: Jarkko Sakkinen @ 2025-04-03 18:45 UTC (permalink / raw)
To: Jonathan McDowell
Cc: Michal Suchanek, Peter Huewe, Jason Gunthorpe, linux-integrity,
linux-kernel
On Wed, Apr 02, 2025 at 06:45:40PM +0100, Jonathan McDowell wrote:
> On Wed, Apr 02, 2025 at 07:21:30PM +0200, Michal Suchanek wrote:
> > With some Infineon chips the timeouts in tpm_tis_send_data (both B and
> > C) can reach up to about 2250 ms.
> >
> > Extend the timeout duration to accommodate this.
>
> The problem here is the bump of timeout_c is going to interact poorly with
> the Infineon errata workaround, as now we'll wait 4s instead of 200ms to
> detect the stuck status change.
>
> (Also shouldn't timeout_c already end up as 750ms, as it's
> max(TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_C), and TIS_SHORT_TIMEOUT is 750 vs 200
> for TPM2_TIMEOUT_C? That doesn't seem to be borne out by your logs, nor my
> results.)
Just noticed that the commit did not end up having fixes etc. tags:
https://web.git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git/commit/?id=de9e33df7762abbfc2a1568291f2c3a3154c6a9d
Should we forward to stable?
BR, Jarkko
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] tpm: tis: Increase the default for timeout B
2025-04-03 18:25 ` [PATCH] tpm: tis: Increase the default for timeout B Michal Suchanek
2025-04-03 18:28 ` Paul Menzel
@ 2025-04-03 18:49 ` Jarkko Sakkinen
2025-04-04 7:53 ` Michal Suchánek
2025-04-04 8:12 ` Michal Suchánek
1 sibling, 2 replies; 26+ messages in thread
From: Jarkko Sakkinen @ 2025-04-03 18:49 UTC (permalink / raw)
To: Michal Suchanek
Cc: Peter Huewe, Jason Gunthorpe, linux-integrity, linux-kernel,
Jonathan McDowell
On Thu, Apr 03, 2025 at 08:25:05PM +0200, Michal Suchanek wrote:
> With some Infineon chips the timeouts in tpm_tis_send_data (both B and
> C) can reach up to about 2250 ms.
>
> Timeout C is retried since
> commit de9e33df7762 ("tpm, tpm_tis: Workaround failed command reception on Infineon devices")
>
> Timeout B still needs to be extended.
>
> Link: https://lore.kernel.org/linux-integrity/Z5pI07m0Muapyu9w@kitsune.suse.cz/
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---
> V2: Only extend timeout B
git format-patch --v2 ;-)
NP, but use --v3 next time...
> ---
> drivers/char/tpm/tpm_tis_core.h | 2 +-
> include/linux/tpm.h | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h
> index 970d02c337c7..c272c25eb9d4 100644
> --- a/drivers/char/tpm/tpm_tis_core.h
> +++ b/drivers/char/tpm/tpm_tis_core.h
> @@ -54,7 +54,7 @@ enum tis_int_flags {
> enum tis_defaults {
> TIS_MEM_LEN = 0x5000,
> TIS_SHORT_TIMEOUT = 750, /* ms */
> - TIS_LONG_TIMEOUT = 2000, /* 2 sec */
> + TIS_LONG_TIMEOUT = 4000, /* 4 sec */
nit: secs (that said, don't care that much)
> TIS_TIMEOUT_MIN_ATML = 14700, /* usecs */
> TIS_TIMEOUT_MAX_ATML = 15000, /* usecs */
> };
> diff --git a/include/linux/tpm.h b/include/linux/tpm.h
> index 6c3125300c00..3db0b6a87d45 100644
> --- a/include/linux/tpm.h
> +++ b/include/linux/tpm.h
> @@ -224,7 +224,7 @@ enum tpm2_const {
>
> enum tpm2_timeouts {
> TPM2_TIMEOUT_A = 750,
> - TPM2_TIMEOUT_B = 2000,
> + TPM2_TIMEOUT_B = 4000,
> TPM2_TIMEOUT_C = 200,
> TPM2_TIMEOUT_D = 30,
> TPM2_DURATION_SHORT = 20,
> --
> 2.47.1
>
Have you tested with:
https://web.git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git/commit/?id=de9e33df7762abbfc2a1568291f2c3a3154c6a9d
?
BR, Jarkko
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] tpm: tis: Increase the default for timeouts B and C
2025-04-03 18:45 ` [PATCH] tpm: tis: Increase the default for timeouts B and C Jarkko Sakkinen
@ 2025-04-03 20:43 ` Jonathan McDowell
2025-04-04 7:51 ` Michal Suchánek
0 siblings, 1 reply; 26+ messages in thread
From: Jonathan McDowell @ 2025-04-03 20:43 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: Michal Suchanek, Peter Huewe, Jason Gunthorpe, linux-integrity,
linux-kernel
On Thu, Apr 03, 2025 at 09:45:21PM +0300, Jarkko Sakkinen wrote:
>On Wed, Apr 02, 2025 at 06:45:40PM +0100, Jonathan McDowell wrote:
>> On Wed, Apr 02, 2025 at 07:21:30PM +0200, Michal Suchanek wrote:
>> > With some Infineon chips the timeouts in tpm_tis_send_data (both B and
>> > C) can reach up to about 2250 ms.
>> >
>> > Extend the timeout duration to accommodate this.
>>
>> The problem here is the bump of timeout_c is going to interact poorly with
>> the Infineon errata workaround, as now we'll wait 4s instead of 200ms to
>> detect the stuck status change.
>>
>> (Also shouldn't timeout_c already end up as 750ms, as it's
>> max(TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_C), and TIS_SHORT_TIMEOUT is 750 vs 200
>> for TPM2_TIMEOUT_C? That doesn't seem to be borne out by your logs, nor my
>> results.)
>
>Just noticed that the commit did not end up having fixes etc. tags:
>
>https://web.git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git/commit/?id=de9e33df7762abbfc2a1568291f2c3a3154c6a9d
>
>Should we forward to stable?
It's a TPM bug rather than a kernel issue, so I don't think there's a
valid Fixes: for it, but it's certainly stable material in my mind.
J.
--
... "It only counts as a lie-in if you don't get dressed before tea time."
-- Steve Willison
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] tpm: tis: Increase the default for timeouts B and C
2025-04-03 20:43 ` Jonathan McDowell
@ 2025-04-04 7:51 ` Michal Suchánek
2025-04-04 8:10 ` Jarkko Sakkinen
0 siblings, 1 reply; 26+ messages in thread
From: Michal Suchánek @ 2025-04-04 7:51 UTC (permalink / raw)
To: Jonathan McDowell
Cc: Jarkko Sakkinen, Peter Huewe, Jason Gunthorpe, linux-integrity,
linux-kernel
On Thu, Apr 03, 2025 at 09:43:19PM +0100, Jonathan McDowell wrote:
> On Thu, Apr 03, 2025 at 09:45:21PM +0300, Jarkko Sakkinen wrote:
> > On Wed, Apr 02, 2025 at 06:45:40PM +0100, Jonathan McDowell wrote:
> > > On Wed, Apr 02, 2025 at 07:21:30PM +0200, Michal Suchanek wrote:
> > > > With some Infineon chips the timeouts in tpm_tis_send_data (both B and
> > > > C) can reach up to about 2250 ms.
> > > >
> > > > Extend the timeout duration to accommodate this.
> > >
> > > The problem here is the bump of timeout_c is going to interact poorly with
> > > the Infineon errata workaround, as now we'll wait 4s instead of 200ms to
> > > detect the stuck status change.
> > >
> > > (Also shouldn't timeout_c already end up as 750ms, as it's
> > > max(TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_C), and TIS_SHORT_TIMEOUT is 750 vs 200
> > > for TPM2_TIMEOUT_C? That doesn't seem to be borne out by your logs, nor my
> > > results.)
> >
> > Just noticed that the commit did not end up having fixes etc. tags:
> >
> > https://web.git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git/commit/?id=de9e33df7762abbfc2a1568291f2c3a3154c6a9d
> >
> > Should we forward to stable?
>
> It's a TPM bug rather than a kernel issue, so I don't think there's a valid
> Fixes: for it, but it's certainly stable material in my mind.
In the more general sense of Fixes: indicating where the fix is
applicable it would be any kernel that supports TPM2.
Thanks
Michal
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] tpm: tis: Increase the default for timeout B
2025-04-03 18:49 ` [PATCH] tpm: tis: Increase the default for timeout B Jarkko Sakkinen
@ 2025-04-04 7:53 ` Michal Suchánek
2025-04-04 8:12 ` Michal Suchánek
1 sibling, 0 replies; 26+ messages in thread
From: Michal Suchánek @ 2025-04-04 7:53 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: Peter Huewe, Jason Gunthorpe, linux-integrity, linux-kernel,
Jonathan McDowell
On Thu, Apr 03, 2025 at 09:49:02PM +0300, Jarkko Sakkinen wrote:
> On Thu, Apr 03, 2025 at 08:25:05PM +0200, Michal Suchanek wrote:
> > With some Infineon chips the timeouts in tpm_tis_send_data (both B and
> > C) can reach up to about 2250 ms.
> >
> > Timeout C is retried since
> > commit de9e33df7762 ("tpm, tpm_tis: Workaround failed command reception on Infineon devices")
> >
> > Timeout B still needs to be extended.
> >
> > Link: https://lore.kernel.org/linux-integrity/Z5pI07m0Muapyu9w@kitsune.suse.cz/
> > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > ---
> > V2: Only extend timeout B
>
> git format-patch --v2 ;-)
>
> NP, but use --v3 next time...
>
> > ---
> > drivers/char/tpm/tpm_tis_core.h | 2 +-
> > include/linux/tpm.h | 2 +-
> > 2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h
> > index 970d02c337c7..c272c25eb9d4 100644
> > --- a/drivers/char/tpm/tpm_tis_core.h
> > +++ b/drivers/char/tpm/tpm_tis_core.h
> > @@ -54,7 +54,7 @@ enum tis_int_flags {
> > enum tis_defaults {
> > TIS_MEM_LEN = 0x5000,
> > TIS_SHORT_TIMEOUT = 750, /* ms */
> > - TIS_LONG_TIMEOUT = 2000, /* 2 sec */
> > + TIS_LONG_TIMEOUT = 4000, /* 4 sec */
>
> nit: secs (that said, don't care that much)
>
> > TIS_TIMEOUT_MIN_ATML = 14700, /* usecs */
> > TIS_TIMEOUT_MAX_ATML = 15000, /* usecs */
> > };
> > diff --git a/include/linux/tpm.h b/include/linux/tpm.h
> > index 6c3125300c00..3db0b6a87d45 100644
> > --- a/include/linux/tpm.h
> > +++ b/include/linux/tpm.h
> > @@ -224,7 +224,7 @@ enum tpm2_const {
> >
> > enum tpm2_timeouts {
> > TPM2_TIMEOUT_A = 750,
> > - TPM2_TIMEOUT_B = 2000,
> > + TPM2_TIMEOUT_B = 4000,
> > TPM2_TIMEOUT_C = 200,
> > TPM2_TIMEOUT_D = 30,
> > TPM2_DURATION_SHORT = 20,
> > --
> > 2.47.1
> >
>
> Have you tested with:
>
> https://web.git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git/commit/?id=de9e33df7762abbfc2a1568291f2c3a3154c6a9d
I haven't. It will take about a week to test if things go well.
Nonetheless, it's fairly clear that both timeouts are exceeded, and this
fix is only for one of them.
Thanks
Michal
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] tpm: tis: Increase the default for timeouts B and C
2025-04-04 7:51 ` Michal Suchánek
@ 2025-04-04 8:10 ` Jarkko Sakkinen
2025-04-04 9:31 ` Jonathan McDowell
0 siblings, 1 reply; 26+ messages in thread
From: Jarkko Sakkinen @ 2025-04-04 8:10 UTC (permalink / raw)
To: Michal Suchánek
Cc: Jonathan McDowell, Peter Huewe, Jason Gunthorpe, linux-integrity,
linux-kernel
On Fri, Apr 04, 2025 at 09:51:29AM +0200, Michal Suchánek wrote:
> On Thu, Apr 03, 2025 at 09:43:19PM +0100, Jonathan McDowell wrote:
> > On Thu, Apr 03, 2025 at 09:45:21PM +0300, Jarkko Sakkinen wrote:
> > > On Wed, Apr 02, 2025 at 06:45:40PM +0100, Jonathan McDowell wrote:
> > > > On Wed, Apr 02, 2025 at 07:21:30PM +0200, Michal Suchanek wrote:
> > > > > With some Infineon chips the timeouts in tpm_tis_send_data (both B and
> > > > > C) can reach up to about 2250 ms.
> > > > >
> > > > > Extend the timeout duration to accommodate this.
> > > >
> > > > The problem here is the bump of timeout_c is going to interact poorly with
> > > > the Infineon errata workaround, as now we'll wait 4s instead of 200ms to
> > > > detect the stuck status change.
> > > >
> > > > (Also shouldn't timeout_c already end up as 750ms, as it's
> > > > max(TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_C), and TIS_SHORT_TIMEOUT is 750 vs 200
> > > > for TPM2_TIMEOUT_C? That doesn't seem to be borne out by your logs, nor my
> > > > results.)
> > >
> > > Just noticed that the commit did not end up having fixes etc. tags:
> > >
> > > https://web.git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git/commit/?id=de9e33df7762abbfc2a1568291f2c3a3154c6a9d
> > >
> > > Should we forward to stable?
> >
> > It's a TPM bug rather than a kernel issue, so I don't think there's a valid
> > Fixes: for it, but it's certainly stable material in my mind.
>
> In the more general sense of Fixes: indicating where the fix is
> applicable it would be any kernel that supports TPM2.
I tried applying the patch on 6.1-stable:
~/work/kernel.org/stable/linux tags/v6.1.132
$ git am -3 ~/Downloads/infineon.patch
Applying: tpm, tpm_tis: Workaround failed command reception on Infineon devices
Using index info to reconstruct a base tree...
M drivers/char/tpm/tpm_tis_core.c
M drivers/char/tpm/tpm_tis_core.h
M include/linux/tpm.h
Falling back to patching base and 3-way merge...
Auto-merging include/linux/tpm.h
Auto-merging drivers/char/tpm/tpm_tis_core.h
Auto-merging drivers/char/tpm/tpm_tis_core.c
If no counter-opinions, I'd add:
stable@vger.kernel.org # v6.1+
I based this on Bookworm kernel.
>
> Thanks
>
> Michal
BR, Jarkko
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] tpm: tis: Increase the default for timeout B
2025-04-03 18:49 ` [PATCH] tpm: tis: Increase the default for timeout B Jarkko Sakkinen
2025-04-04 7:53 ` Michal Suchánek
@ 2025-04-04 8:12 ` Michal Suchánek
2025-04-04 8:14 ` Jarkko Sakkinen
1 sibling, 1 reply; 26+ messages in thread
From: Michal Suchánek @ 2025-04-04 8:12 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: Peter Huewe, Jason Gunthorpe, linux-integrity, linux-kernel,
Jonathan McDowell
On Thu, Apr 03, 2025 at 09:49:02PM +0300, Jarkko Sakkinen wrote:
> On Thu, Apr 03, 2025 at 08:25:05PM +0200, Michal Suchanek wrote:
> > With some Infineon chips the timeouts in tpm_tis_send_data (both B and
> > C) can reach up to about 2250 ms.
> >
> > Timeout C is retried since
> > commit de9e33df7762 ("tpm, tpm_tis: Workaround failed command reception on Infineon devices")
> >
> > Timeout B still needs to be extended.
> >
> > Link: https://lore.kernel.org/linux-integrity/Z5pI07m0Muapyu9w@kitsune.suse.cz/
> > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > ---
> > V2: Only extend timeout B
>
> git format-patch --v2 ;-)
>
> NP, but use --v3 next time...
Where do you get git with such practical options?
It does not seem to be supported by the upstream version.
Thanks
Michal
>
> > ---
> > drivers/char/tpm/tpm_tis_core.h | 2 +-
> > include/linux/tpm.h | 2 +-
> > 2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h
> > index 970d02c337c7..c272c25eb9d4 100644
> > --- a/drivers/char/tpm/tpm_tis_core.h
> > +++ b/drivers/char/tpm/tpm_tis_core.h
> > @@ -54,7 +54,7 @@ enum tis_int_flags {
> > enum tis_defaults {
> > TIS_MEM_LEN = 0x5000,
> > TIS_SHORT_TIMEOUT = 750, /* ms */
> > - TIS_LONG_TIMEOUT = 2000, /* 2 sec */
> > + TIS_LONG_TIMEOUT = 4000, /* 4 sec */
>
> nit: secs (that said, don't care that much)
>
> > TIS_TIMEOUT_MIN_ATML = 14700, /* usecs */
> > TIS_TIMEOUT_MAX_ATML = 15000, /* usecs */
> > };
> > diff --git a/include/linux/tpm.h b/include/linux/tpm.h
> > index 6c3125300c00..3db0b6a87d45 100644
> > --- a/include/linux/tpm.h
> > +++ b/include/linux/tpm.h
> > @@ -224,7 +224,7 @@ enum tpm2_const {
> >
> > enum tpm2_timeouts {
> > TPM2_TIMEOUT_A = 750,
> > - TPM2_TIMEOUT_B = 2000,
> > + TPM2_TIMEOUT_B = 4000,
> > TPM2_TIMEOUT_C = 200,
> > TPM2_TIMEOUT_D = 30,
> > TPM2_DURATION_SHORT = 20,
> > --
> > 2.47.1
> >
>
> Have you tested with:
>
> https://web.git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git/commit/?id=de9e33df7762abbfc2a1568291f2c3a3154c6a9d
>
> ?
>
> BR, Jarkko
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] tpm: tis: Increase the default for timeout B
2025-04-04 8:12 ` Michal Suchánek
@ 2025-04-04 8:14 ` Jarkko Sakkinen
0 siblings, 0 replies; 26+ messages in thread
From: Jarkko Sakkinen @ 2025-04-04 8:14 UTC (permalink / raw)
To: Michal Suchánek
Cc: Peter Huewe, Jason Gunthorpe, linux-integrity, linux-kernel,
Jonathan McDowell
On Fri, Apr 04, 2025 at 10:12:18AM +0200, Michal Suchánek wrote:
> On Thu, Apr 03, 2025 at 09:49:02PM +0300, Jarkko Sakkinen wrote:
> > On Thu, Apr 03, 2025 at 08:25:05PM +0200, Michal Suchanek wrote:
> > > With some Infineon chips the timeouts in tpm_tis_send_data (both B and
> > > C) can reach up to about 2250 ms.
> > >
> > > Timeout C is retried since
> > > commit de9e33df7762 ("tpm, tpm_tis: Workaround failed command reception on Infineon devices")
> > >
> > > Timeout B still needs to be extended.
> > >
> > > Link: https://lore.kernel.org/linux-integrity/Z5pI07m0Muapyu9w@kitsune.suse.cz/
> > > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > > ---
> > > V2: Only extend timeout B
> >
> > git format-patch --v2 ;-)
> >
> > NP, but use --v3 next time...
>
> Where do you get git with such practical options?
Oops! My bad, sorry.
$ git format-patch --v2
fatal: unrecognized argument: --v2
~/work/kernel.org/stable/linux 2254ea2ccee8
$ git format-patch -v2
# success
BR, Jarkko
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v3] tpm: tis: Double the timeout B to 4s
2025-04-03 18:28 ` Paul Menzel
@ 2025-04-04 8:23 ` Michal Suchanek
2025-04-04 8:53 ` Jarkko Sakkinen
0 siblings, 1 reply; 26+ messages in thread
From: Michal Suchanek @ 2025-04-04 8:23 UTC (permalink / raw)
Cc: Michal Suchanek, Peter Huewe, Jarkko Sakkinen, Jason Gunthorpe,
linux-integrity, linux-kernel, Jonathan McDowell, Paul Menzel
With some Infineon chips the timeouts in tpm_tis_send_data (both B and
C) can reach up to about 2250 ms.
Timeout C is retried since
commit de9e33df7762 ("tpm, tpm_tis: Workaround failed command reception on Infineon devices")
Timeout B still needs to be extended.
The problem is most commonly encountered with context related operation
such as load context/save context. These are issued directly by the
kernel, and there is no retry logic for them.
When a filesystem is set up to use the TPM for unlocking the boot fails,
and restarting the userspace service is ineffective. This is likely
because ignoring a load context/save context result puts the real TPM
state and the TPM state expected by the kernel out of sync.
Chips known to be affected:
tpm_tis IFX1522:00: 2.0 TPM (device-id 0x1D, rev-id 54)
Description: SLB9672
Firmware Revision: 15.22
tpm_tis MSFT0101:00: 2.0 TPM (device-id 0x1B, rev-id 22)
Firmware Revision: 7.83
tpm_tis MSFT0101:00: 2.0 TPM (device-id 0x1A, rev-id 16)
Firmware Revision: 5.63
Link: https://lore.kernel.org/linux-integrity/Z5pI07m0Muapyu9w@kitsune.suse.cz/
Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
v2: Only extend timeout B
v3: Update commit message
---
drivers/char/tpm/tpm_tis_core.h | 2 +-
include/linux/tpm.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h
index 970d02c337c7..6c3aa480396b 100644
--- a/drivers/char/tpm/tpm_tis_core.h
+++ b/drivers/char/tpm/tpm_tis_core.h
@@ -54,7 +54,7 @@ enum tis_int_flags {
enum tis_defaults {
TIS_MEM_LEN = 0x5000,
TIS_SHORT_TIMEOUT = 750, /* ms */
- TIS_LONG_TIMEOUT = 2000, /* 2 sec */
+ TIS_LONG_TIMEOUT = 4000, /* 4 secs */
TIS_TIMEOUT_MIN_ATML = 14700, /* usecs */
TIS_TIMEOUT_MAX_ATML = 15000, /* usecs */
};
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index 6c3125300c00..3db0b6a87d45 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -224,7 +224,7 @@ enum tpm2_const {
enum tpm2_timeouts {
TPM2_TIMEOUT_A = 750,
- TPM2_TIMEOUT_B = 2000,
+ TPM2_TIMEOUT_B = 4000,
TPM2_TIMEOUT_C = 200,
TPM2_TIMEOUT_D = 30,
TPM2_DURATION_SHORT = 20,
--
2.47.1
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH v3] tpm: tis: Double the timeout B to 4s
2025-04-04 8:23 ` [PATCH v3] tpm: tis: Double the timeout B to 4s Michal Suchanek
@ 2025-04-04 8:53 ` Jarkko Sakkinen
2025-05-14 12:10 ` Michal Suchánek
0 siblings, 1 reply; 26+ messages in thread
From: Jarkko Sakkinen @ 2025-04-04 8:53 UTC (permalink / raw)
To: Michal Suchanek
Cc: Peter Huewe, Jason Gunthorpe, linux-integrity, linux-kernel,
Jonathan McDowell, Paul Menzel
On Fri, Apr 04, 2025 at 10:23:14AM +0200, Michal Suchanek wrote:
> With some Infineon chips the timeouts in tpm_tis_send_data (both B and
> C) can reach up to about 2250 ms.
>
> Timeout C is retried since
> commit de9e33df7762 ("tpm, tpm_tis: Workaround failed command reception on Infineon devices")
>
> Timeout B still needs to be extended.
>
> The problem is most commonly encountered with context related operation
> such as load context/save context. These are issued directly by the
> kernel, and there is no retry logic for them.
>
> When a filesystem is set up to use the TPM for unlocking the boot fails,
> and restarting the userspace service is ineffective. This is likely
> because ignoring a load context/save context result puts the real TPM
> state and the TPM state expected by the kernel out of sync.
>
> Chips known to be affected:
> tpm_tis IFX1522:00: 2.0 TPM (device-id 0x1D, rev-id 54)
> Description: SLB9672
> Firmware Revision: 15.22
>
> tpm_tis MSFT0101:00: 2.0 TPM (device-id 0x1B, rev-id 22)
> Firmware Revision: 7.83
>
> tpm_tis MSFT0101:00: 2.0 TPM (device-id 0x1A, rev-id 16)
> Firmware Revision: 5.63
>
> Link: https://lore.kernel.org/linux-integrity/Z5pI07m0Muapyu9w@kitsune.suse.cz/
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---
> v2: Only extend timeout B
> v3: Update commit message
> ---
> drivers/char/tpm/tpm_tis_core.h | 2 +-
> include/linux/tpm.h | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h
> index 970d02c337c7..6c3aa480396b 100644
> --- a/drivers/char/tpm/tpm_tis_core.h
> +++ b/drivers/char/tpm/tpm_tis_core.h
> @@ -54,7 +54,7 @@ enum tis_int_flags {
> enum tis_defaults {
> TIS_MEM_LEN = 0x5000,
> TIS_SHORT_TIMEOUT = 750, /* ms */
> - TIS_LONG_TIMEOUT = 2000, /* 2 sec */
> + TIS_LONG_TIMEOUT = 4000, /* 4 secs */
> TIS_TIMEOUT_MIN_ATML = 14700, /* usecs */
> TIS_TIMEOUT_MAX_ATML = 15000, /* usecs */
> };
> diff --git a/include/linux/tpm.h b/include/linux/tpm.h
> index 6c3125300c00..3db0b6a87d45 100644
> --- a/include/linux/tpm.h
> +++ b/include/linux/tpm.h
> @@ -224,7 +224,7 @@ enum tpm2_const {
>
> enum tpm2_timeouts {
> TPM2_TIMEOUT_A = 750,
> - TPM2_TIMEOUT_B = 2000,
> + TPM2_TIMEOUT_B = 4000,
> TPM2_TIMEOUT_C = 200,
> TPM2_TIMEOUT_D = 30,
> TPM2_DURATION_SHORT = 20,
> --
> 2.47.1
>
>
Cc: stable@vger.kernel.org # v6.1+
Probably best that I'll piggyback a patch set for stable with the two
fixes, in order to cause least noise. I need to do this *after* an
ack'd PR to -rc2.
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
BR, Jarkko
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] tpm: tis: Increase the default for timeouts B and C
2025-04-04 8:10 ` Jarkko Sakkinen
@ 2025-04-04 9:31 ` Jonathan McDowell
2025-04-04 11:58 ` Jarkko Sakkinen
0 siblings, 1 reply; 26+ messages in thread
From: Jonathan McDowell @ 2025-04-04 9:31 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: Michal Suchánek, Peter Huewe, Jason Gunthorpe,
linux-integrity, linux-kernel, Sasha Levin
On Fri, Apr 04, 2025 at 11:10:12AM +0300, Jarkko Sakkinen wrote:
>On Fri, Apr 04, 2025 at 09:51:29AM +0200, Michal Suchánek wrote:
>> On Thu, Apr 03, 2025 at 09:43:19PM +0100, Jonathan McDowell wrote:
>> > On Thu, Apr 03, 2025 at 09:45:21PM +0300, Jarkko Sakkinen wrote:
>> > > On Wed, Apr 02, 2025 at 06:45:40PM +0100, Jonathan McDowell wrote:
>> > > > On Wed, Apr 02, 2025 at 07:21:30PM +0200, Michal Suchanek wrote:
>> > > > > With some Infineon chips the timeouts in tpm_tis_send_data (both B and
>> > > > > C) can reach up to about 2250 ms.
>> > > > >
>> > > > > Extend the timeout duration to accommodate this.
>> > > >
>> > > > The problem here is the bump of timeout_c is going to interact poorly with
>> > > > the Infineon errata workaround, as now we'll wait 4s instead of 200ms to
>> > > > detect the stuck status change.
>> > > >
>> > > > (Also shouldn't timeout_c already end up as 750ms, as it's
>> > > > max(TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_C), and TIS_SHORT_TIMEOUT is 750 vs 200
>> > > > for TPM2_TIMEOUT_C? That doesn't seem to be borne out by your logs, nor my
>> > > > results.)
>> > >
>> > > Just noticed that the commit did not end up having fixes etc. tags:
>> > >
>> > > https://web.git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git/commit/?id=de9e33df7762abbfc2a1568291f2c3a3154c6a9d
>> > >
>> > > Should we forward to stable?
>> >
>> > It's a TPM bug rather than a kernel issue, so I don't think there's a valid
>> > Fixes: for it, but it's certainly stable material in my mind.
>>
>> In the more general sense of Fixes: indicating where the fix is
>> applicable it would be any kernel that supports TPM2.
>
>I tried applying the patch on 6.1-stable:
>
>~/work/kernel.org/stable/linux tags/v6.1.132
>$ git am -3 ~/Downloads/infineon.patch
>Applying: tpm, tpm_tis: Workaround failed command reception on Infineon devices
>Using index info to reconstruct a base tree...
>M drivers/char/tpm/tpm_tis_core.c
>M drivers/char/tpm/tpm_tis_core.h
>M include/linux/tpm.h
>Falling back to patching base and 3-way merge...
>Auto-merging include/linux/tpm.h
>Auto-merging drivers/char/tpm/tpm_tis_core.h
>Auto-merging drivers/char/tpm/tpm_tis_core.c
>
>If no counter-opinions, I'd add:
>
>stable@vger.kernel.org # v6.1+
>
>I based this on Bookworm kernel.
It looks like Sasha has already autoselected it for 6.1, 6.6, 6.12, 6.13
+ 6.14.
J.
--
How does it work? I don't know but it does!
This .sig brought to you by the letter R and the number 21
Product of the Republic of HuggieTag
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] tpm: tis: Increase the default for timeouts B and C
2025-04-04 9:31 ` Jonathan McDowell
@ 2025-04-04 11:58 ` Jarkko Sakkinen
0 siblings, 0 replies; 26+ messages in thread
From: Jarkko Sakkinen @ 2025-04-04 11:58 UTC (permalink / raw)
To: Jonathan McDowell, Sasha Levin
Cc: Michal Suchánek, Peter Huewe, Jason Gunthorpe,
linux-integrity, linux-kernel, Sasha Levin
On Fri, Apr 04, 2025 at 10:31:18AM +0100, Jonathan McDowell wrote:
> On Fri, Apr 04, 2025 at 11:10:12AM +0300, Jarkko Sakkinen wrote:
> > On Fri, Apr 04, 2025 at 09:51:29AM +0200, Michal Suchánek wrote:
> > > On Thu, Apr 03, 2025 at 09:43:19PM +0100, Jonathan McDowell wrote:
> > > > On Thu, Apr 03, 2025 at 09:45:21PM +0300, Jarkko Sakkinen wrote:
> > > > > On Wed, Apr 02, 2025 at 06:45:40PM +0100, Jonathan McDowell wrote:
> > > > > > On Wed, Apr 02, 2025 at 07:21:30PM +0200, Michal Suchanek wrote:
> > > > > > > With some Infineon chips the timeouts in tpm_tis_send_data (both B and
> > > > > > > C) can reach up to about 2250 ms.
> > > > > > >
> > > > > > > Extend the timeout duration to accommodate this.
> > > > > >
> > > > > > The problem here is the bump of timeout_c is going to interact poorly with
> > > > > > the Infineon errata workaround, as now we'll wait 4s instead of 200ms to
> > > > > > detect the stuck status change.
> > > > > >
> > > > > > (Also shouldn't timeout_c already end up as 750ms, as it's
> > > > > > max(TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_C), and TIS_SHORT_TIMEOUT is 750 vs 200
> > > > > > for TPM2_TIMEOUT_C? That doesn't seem to be borne out by your logs, nor my
> > > > > > results.)
> > > > >
> > > > > Just noticed that the commit did not end up having fixes etc. tags:
> > > > >
> > > > > https://web.git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git/commit/?id=de9e33df7762abbfc2a1568291f2c3a3154c6a9d
> > > > >
> > > > > Should we forward to stable?
> > > >
> > > > It's a TPM bug rather than a kernel issue, so I don't think there's a valid
> > > > Fixes: for it, but it's certainly stable material in my mind.
> > >
> > > In the more general sense of Fixes: indicating where the fix is
> > > applicable it would be any kernel that supports TPM2.
> >
> > I tried applying the patch on 6.1-stable:
> >
> > ~/work/kernel.org/stable/linux tags/v6.1.132
> > $ git am -3 ~/Downloads/infineon.patch
> > Applying: tpm, tpm_tis: Workaround failed command reception on Infineon devices
> > Using index info to reconstruct a base tree...
> > M drivers/char/tpm/tpm_tis_core.c
> > M drivers/char/tpm/tpm_tis_core.h
> > M include/linux/tpm.h
> > Falling back to patching base and 3-way merge...
> > Auto-merging include/linux/tpm.h
> > Auto-merging drivers/char/tpm/tpm_tis_core.h
> > Auto-merging drivers/char/tpm/tpm_tis_core.c
> >
> > If no counter-opinions, I'd add:
> >
> > stable@vger.kernel.org # v6.1+
> >
> > I based this on Bookworm kernel.
>
> It looks like Sasha has already autoselected it for 6.1, 6.6, 6.12, 6.13 +
> 6.14.
Right! I can see also those mails, and exactly the version range I would
have proposed :-) Perfect, thanks Sasha!
>
> J.
>
> --
> How does it work? I don't know but it does!
> This .sig brought to you by the letter R and the number 21
> Product of the Republic of HuggieTag
>
BR, Jarkko
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3] tpm: tis: Double the timeout B to 4s
2025-04-04 8:53 ` Jarkko Sakkinen
@ 2025-05-14 12:10 ` Michal Suchánek
2025-05-15 1:41 ` Jarkko Sakkinen
0 siblings, 1 reply; 26+ messages in thread
From: Michal Suchánek @ 2025-05-14 12:10 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: Peter Huewe, Jason Gunthorpe, linux-integrity, linux-kernel,
Jonathan McDowell, Paul Menzel
Hello,
On Fri, Apr 04, 2025 at 11:53:00AM +0300, Jarkko Sakkinen wrote:
> On Fri, Apr 04, 2025 at 10:23:14AM +0200, Michal Suchanek wrote:
> > With some Infineon chips the timeouts in tpm_tis_send_data (both B and
> > C) can reach up to about 2250 ms.
> >
> > Timeout C is retried since
> > commit de9e33df7762 ("tpm, tpm_tis: Workaround failed command reception on Infineon devices")
> >
> > Timeout B still needs to be extended.
> >
> > The problem is most commonly encountered with context related operation
> > such as load context/save context. These are issued directly by the
> > kernel, and there is no retry logic for them.
> >
> > When a filesystem is set up to use the TPM for unlocking the boot fails,
> > and restarting the userspace service is ineffective. This is likely
> > because ignoring a load context/save context result puts the real TPM
> > state and the TPM state expected by the kernel out of sync.
> >
> > Chips known to be affected:
> > tpm_tis IFX1522:00: 2.0 TPM (device-id 0x1D, rev-id 54)
> > Description: SLB9672
> > Firmware Revision: 15.22
> >
> > tpm_tis MSFT0101:00: 2.0 TPM (device-id 0x1B, rev-id 22)
> > Firmware Revision: 7.83
> >
> > tpm_tis MSFT0101:00: 2.0 TPM (device-id 0x1A, rev-id 16)
> > Firmware Revision: 5.63
> >
> > Link: https://lore.kernel.org/linux-integrity/Z5pI07m0Muapyu9w@kitsune.suse.cz/
> > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > ---
> > v2: Only extend timeout B
> > v3: Update commit message
> > ---
> > drivers/char/tpm/tpm_tis_core.h | 2 +-
> > include/linux/tpm.h | 2 +-
> > 2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h
> > index 970d02c337c7..6c3aa480396b 100644
> > --- a/drivers/char/tpm/tpm_tis_core.h
> > +++ b/drivers/char/tpm/tpm_tis_core.h
> > @@ -54,7 +54,7 @@ enum tis_int_flags {
> > enum tis_defaults {
> > TIS_MEM_LEN = 0x5000,
> > TIS_SHORT_TIMEOUT = 750, /* ms */
> > - TIS_LONG_TIMEOUT = 2000, /* 2 sec */
> > + TIS_LONG_TIMEOUT = 4000, /* 4 secs */
> > TIS_TIMEOUT_MIN_ATML = 14700, /* usecs */
> > TIS_TIMEOUT_MAX_ATML = 15000, /* usecs */
> > };
> > diff --git a/include/linux/tpm.h b/include/linux/tpm.h
> > index 6c3125300c00..3db0b6a87d45 100644
> > --- a/include/linux/tpm.h
> > +++ b/include/linux/tpm.h
> > @@ -224,7 +224,7 @@ enum tpm2_const {
> >
> > enum tpm2_timeouts {
> > TPM2_TIMEOUT_A = 750,
> > - TPM2_TIMEOUT_B = 2000,
> > + TPM2_TIMEOUT_B = 4000,
> > TPM2_TIMEOUT_C = 200,
> > TPM2_TIMEOUT_D = 30,
> > TPM2_DURATION_SHORT = 20,
> > --
> > 2.47.1
> >
> >
>
> Cc: stable@vger.kernel.org # v6.1+
>
> Probably best that I'll piggyback a patch set for stable with the two
> fixes, in order to cause least noise. I need to do this *after* an
> ack'd PR to -rc2.
While there is talk about stable this does not seem to be applied
anywhere I could find. Is that expected?
Thanks
Michal
>
> Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
>
> BR, Jarkko
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3] tpm: tis: Double the timeout B to 4s
2025-05-14 12:10 ` Michal Suchánek
@ 2025-05-15 1:41 ` Jarkko Sakkinen
2025-05-15 11:17 ` Jarkko Sakkinen
0 siblings, 1 reply; 26+ messages in thread
From: Jarkko Sakkinen @ 2025-05-15 1:41 UTC (permalink / raw)
To: Michal Suchánek, Sasha Levin
Cc: Peter Huewe, Jason Gunthorpe, linux-integrity, linux-kernel,
Jonathan McDowell, Paul Menzel
On Wed, May 14, 2025 at 02:10:45PM +0200, Michal Suchánek wrote:
> Hello,
>
> On Fri, Apr 04, 2025 at 11:53:00AM +0300, Jarkko Sakkinen wrote:
> > On Fri, Apr 04, 2025 at 10:23:14AM +0200, Michal Suchanek wrote:
> > > With some Infineon chips the timeouts in tpm_tis_send_data (both B and
> > > C) can reach up to about 2250 ms.
> > >
> > > Timeout C is retried since
> > > commit de9e33df7762 ("tpm, tpm_tis: Workaround failed command reception on Infineon devices")
> > >
> > > Timeout B still needs to be extended.
> > >
> > > The problem is most commonly encountered with context related operation
> > > such as load context/save context. These are issued directly by the
> > > kernel, and there is no retry logic for them.
> > >
> > > When a filesystem is set up to use the TPM for unlocking the boot fails,
> > > and restarting the userspace service is ineffective. This is likely
> > > because ignoring a load context/save context result puts the real TPM
> > > state and the TPM state expected by the kernel out of sync.
> > >
> > > Chips known to be affected:
> > > tpm_tis IFX1522:00: 2.0 TPM (device-id 0x1D, rev-id 54)
> > > Description: SLB9672
> > > Firmware Revision: 15.22
> > >
> > > tpm_tis MSFT0101:00: 2.0 TPM (device-id 0x1B, rev-id 22)
> > > Firmware Revision: 7.83
> > >
> > > tpm_tis MSFT0101:00: 2.0 TPM (device-id 0x1A, rev-id 16)
> > > Firmware Revision: 5.63
> > >
> > > Link: https://lore.kernel.org/linux-integrity/Z5pI07m0Muapyu9w@kitsune.suse.cz/
> > > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > > ---
> > > v2: Only extend timeout B
> > > v3: Update commit message
> > > ---
> > > drivers/char/tpm/tpm_tis_core.h | 2 +-
> > > include/linux/tpm.h | 2 +-
> > > 2 files changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h
> > > index 970d02c337c7..6c3aa480396b 100644
> > > --- a/drivers/char/tpm/tpm_tis_core.h
> > > +++ b/drivers/char/tpm/tpm_tis_core.h
> > > @@ -54,7 +54,7 @@ enum tis_int_flags {
> > > enum tis_defaults {
> > > TIS_MEM_LEN = 0x5000,
> > > TIS_SHORT_TIMEOUT = 750, /* ms */
> > > - TIS_LONG_TIMEOUT = 2000, /* 2 sec */
> > > + TIS_LONG_TIMEOUT = 4000, /* 4 secs */
> > > TIS_TIMEOUT_MIN_ATML = 14700, /* usecs */
> > > TIS_TIMEOUT_MAX_ATML = 15000, /* usecs */
> > > };
> > > diff --git a/include/linux/tpm.h b/include/linux/tpm.h
> > > index 6c3125300c00..3db0b6a87d45 100644
> > > --- a/include/linux/tpm.h
> > > +++ b/include/linux/tpm.h
> > > @@ -224,7 +224,7 @@ enum tpm2_const {
> > >
> > > enum tpm2_timeouts {
> > > TPM2_TIMEOUT_A = 750,
> > > - TPM2_TIMEOUT_B = 2000,
> > > + TPM2_TIMEOUT_B = 4000,
> > > TPM2_TIMEOUT_C = 200,
> > > TPM2_TIMEOUT_D = 30,
> > > TPM2_DURATION_SHORT = 20,
> > > --
> > > 2.47.1
> > >
> > >
> >
> > Cc: stable@vger.kernel.org # v6.1+
> >
> > Probably best that I'll piggyback a patch set for stable with the two
> > fixes, in order to cause least noise. I need to do this *after* an
> > ack'd PR to -rc2.
>
> While there is talk about stable this does not seem to be applied
> anywhere I could find. Is that expected?
Definitely not. I got shifted away with other work early April and
this was left to my TODO folder, apologies.
Sasha, can you also auto-select this to v6.1+? It is in my next
branch now (should be soon'ish mirrored to linux-next).
BR, Jarkko
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3] tpm: tis: Double the timeout B to 4s
2025-05-15 1:41 ` Jarkko Sakkinen
@ 2025-05-15 11:17 ` Jarkko Sakkinen
0 siblings, 0 replies; 26+ messages in thread
From: Jarkko Sakkinen @ 2025-05-15 11:17 UTC (permalink / raw)
To: Michal Suchánek, Sasha Levin
Cc: Peter Huewe, Jason Gunthorpe, linux-integrity, linux-kernel,
Jonathan McDowell, Paul Menzel
On Thu, May 15, 2025 at 04:41:52AM +0300, Jarkko Sakkinen wrote:
> On Wed, May 14, 2025 at 02:10:45PM +0200, Michal Suchánek wrote:
> > Hello,
> >
> > On Fri, Apr 04, 2025 at 11:53:00AM +0300, Jarkko Sakkinen wrote:
> > > On Fri, Apr 04, 2025 at 10:23:14AM +0200, Michal Suchanek wrote:
> > > > With some Infineon chips the timeouts in tpm_tis_send_data (both B and
> > > > C) can reach up to about 2250 ms.
> > > >
> > > > Timeout C is retried since
> > > > commit de9e33df7762 ("tpm, tpm_tis: Workaround failed command reception on Infineon devices")
> > > >
> > > > Timeout B still needs to be extended.
> > > >
> > > > The problem is most commonly encountered with context related operation
> > > > such as load context/save context. These are issued directly by the
> > > > kernel, and there is no retry logic for them.
> > > >
> > > > When a filesystem is set up to use the TPM for unlocking the boot fails,
> > > > and restarting the userspace service is ineffective. This is likely
> > > > because ignoring a load context/save context result puts the real TPM
> > > > state and the TPM state expected by the kernel out of sync.
> > > >
> > > > Chips known to be affected:
> > > > tpm_tis IFX1522:00: 2.0 TPM (device-id 0x1D, rev-id 54)
> > > > Description: SLB9672
> > > > Firmware Revision: 15.22
> > > >
> > > > tpm_tis MSFT0101:00: 2.0 TPM (device-id 0x1B, rev-id 22)
> > > > Firmware Revision: 7.83
> > > >
> > > > tpm_tis MSFT0101:00: 2.0 TPM (device-id 0x1A, rev-id 16)
> > > > Firmware Revision: 5.63
> > > >
> > > > Link: https://lore.kernel.org/linux-integrity/Z5pI07m0Muapyu9w@kitsune.suse.cz/
> > > > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > > > ---
> > > > v2: Only extend timeout B
> > > > v3: Update commit message
> > > > ---
> > > > drivers/char/tpm/tpm_tis_core.h | 2 +-
> > > > include/linux/tpm.h | 2 +-
> > > > 2 files changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h
> > > > index 970d02c337c7..6c3aa480396b 100644
> > > > --- a/drivers/char/tpm/tpm_tis_core.h
> > > > +++ b/drivers/char/tpm/tpm_tis_core.h
> > > > @@ -54,7 +54,7 @@ enum tis_int_flags {
> > > > enum tis_defaults {
> > > > TIS_MEM_LEN = 0x5000,
> > > > TIS_SHORT_TIMEOUT = 750, /* ms */
> > > > - TIS_LONG_TIMEOUT = 2000, /* 2 sec */
> > > > + TIS_LONG_TIMEOUT = 4000, /* 4 secs */
> > > > TIS_TIMEOUT_MIN_ATML = 14700, /* usecs */
> > > > TIS_TIMEOUT_MAX_ATML = 15000, /* usecs */
> > > > };
> > > > diff --git a/include/linux/tpm.h b/include/linux/tpm.h
> > > > index 6c3125300c00..3db0b6a87d45 100644
> > > > --- a/include/linux/tpm.h
> > > > +++ b/include/linux/tpm.h
> > > > @@ -224,7 +224,7 @@ enum tpm2_const {
> > > >
> > > > enum tpm2_timeouts {
> > > > TPM2_TIMEOUT_A = 750,
> > > > - TPM2_TIMEOUT_B = 2000,
> > > > + TPM2_TIMEOUT_B = 4000,
> > > > TPM2_TIMEOUT_C = 200,
> > > > TPM2_TIMEOUT_D = 30,
> > > > TPM2_DURATION_SHORT = 20,
> > > > --
> > > > 2.47.1
> > > >
> > > >
> > >
> > > Cc: stable@vger.kernel.org # v6.1+
> > >
> > > Probably best that I'll piggyback a patch set for stable with the two
> > > fixes, in order to cause least noise. I need to do this *after* an
> > > ack'd PR to -rc2.
> >
> > While there is talk about stable this does not seem to be applied
> > anywhere I could find. Is that expected?
>
> Definitely not. I got shifted away with other work early April and
> this was left to my TODO folder, apologies.
>
> Sasha, can you also auto-select this to v6.1+? It is in my next
> branch now (should be soon'ish mirrored to linux-next).
I got shifted away at work for a while and since it has been a while,
and the thread is a bit messy, can you check if there was still
something else I ought to pick up:
https://lore.kernel.org/linux-integrity/D9WD3016M557.1ZXO3GLKGUIIF@kernel.org/
Now "tpm: tis: Double the timeout B to 4s" has a legit commit ID at
least, and will land to 6.15.
BR, Jarkko
^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2025-05-15 11:17 UTC | newest]
Thread overview: 26+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-02 17:21 [PATCH] tpm: tis: Increase the default for timeouts B and C Michal Suchanek
2025-04-02 17:45 ` Jonathan McDowell
2025-04-02 20:07 ` Michal Suchánek
2025-04-03 9:31 ` Michal Suchánek
2025-04-03 11:00 ` Jonathan McDowell
2025-04-03 11:56 ` Michal Suchánek
2025-04-03 13:00 ` Jonathan McDowell
2025-04-03 14:11 ` Michal Suchánek
2025-04-03 18:25 ` [PATCH] tpm: tis: Increase the default for timeout B Michal Suchanek
2025-04-03 18:28 ` Paul Menzel
2025-04-04 8:23 ` [PATCH v3] tpm: tis: Double the timeout B to 4s Michal Suchanek
2025-04-04 8:53 ` Jarkko Sakkinen
2025-05-14 12:10 ` Michal Suchánek
2025-05-15 1:41 ` Jarkko Sakkinen
2025-05-15 11:17 ` Jarkko Sakkinen
2025-04-03 18:49 ` [PATCH] tpm: tis: Increase the default for timeout B Jarkko Sakkinen
2025-04-04 7:53 ` Michal Suchánek
2025-04-04 8:12 ` Michal Suchánek
2025-04-04 8:14 ` Jarkko Sakkinen
2025-04-03 18:45 ` [PATCH] tpm: tis: Increase the default for timeouts B and C Jarkko Sakkinen
2025-04-03 20:43 ` Jonathan McDowell
2025-04-04 7:51 ` Michal Suchánek
2025-04-04 8:10 ` Jarkko Sakkinen
2025-04-04 9:31 ` Jonathan McDowell
2025-04-04 11:58 ` Jarkko Sakkinen
2025-04-03 18:38 ` Jarkko Sakkinen
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).