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/z5pi07m0muapy...@kitsune.suse.cz/ > > > Signed-off-by: Michal Suchanek <msucha...@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.
