drivers/char/tpm/tpm_tis_core.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
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.1On 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
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.
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
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
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
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
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
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
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.
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. \- |
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. > \- |
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?
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
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.1On 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
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
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
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
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
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.1On 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
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
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
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
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.
© 2016 - 2026 Red Hat, Inc.