tpm, tpm_tis: Workaround failed command reception on Infineon devices

PATCH Superseded Jonathan McDowell 3 participants 9 messages Mar 6, 2025 – Mar 22, 2025

Subsystems (2)

Keys-trusted TPM device driver

lore

Share & tools

lore

Jonathan McDowell

author

2025-03-06 09:01 UTC #0 lore raw

PATCH 1/1 tpm, tpm_tis: Workaround failed command reception on Infineon devices

From: Jonathan McDowell <[email protected]>

Some Infineon devices have a issue where the status register will get stuck with a quick REQUEST_USE / COMMAND_READY sequence. This is not simply a matter of requiring a longer timeout; the work around is to retry the command submission. Add appropriate logic to do this in the send path.

This is fixed in later firmware revisions, but those are not always available, and cannot generally be easily updated from outside a firmware environment.

Testing has been performed with a simple repeated loop of doing a TPM2_CC_GET_CAPABILITY for TPM_CAP_PROP_MANUFACTURER using the Go code at:

https://the.earth.li/~noodles/tpm-stuff/timeout-reproducer-simple.go

It can take several hours to reproduce, and millions of operations.

Signed-off-by Jonathan McDowell <[email protected]>

---
 drivers/char/tpm/tpm_tis_core.c | 17 ++++++++++++++---
 drivers/char/tpm/tpm_tis_core.h |  1 +
 include/linux/tpm.h             |  1 +
 3 files changed, 16 insertions(+), 3 deletions(-)

drivers/char/tpm/tpm_tis_core.c +14 -3

index 167d71747666..e4eae206a353 100644
@@ -464,7 +464,10 @@ static int tpm_tis_send_data(struct tpm_chip *chip, const u8 *buf, size_t len)
 
 		if (wait_for_tpm_stat(chip, TPM_STS_VALID, chip->timeout_c,
 					&priv->int_queue, false) < 0) {
-			rc = -ETIME;
+			if (test_bit(TPM_TIS_STATUS_WORKAROUND, &priv->flags))
+				rc = -EAGAIN;
+			else
+				rc = -ETIME;
 			goto out_err;
 		}
 		status = tpm_tis_status(chip);

@@ -481,7 +484,10 @@ static int tpm_tis_send_data(struct tpm_chip *chip, const u8 *buf, size_t len)
 
 	if (wait_for_tpm_stat(chip, TPM_STS_VALID, chip->timeout_c,
 				&priv->int_queue, false) < 0) {
-		rc = -ETIME;
+		if (test_bit(TPM_TIS_STATUS_WORKAROUND, &priv->flags))
+			rc = -EAGAIN;
+		else
+			rc = -ETIME;
 		goto out_err;
 	}
 	status = tpm_tis_status(chip);

@@ -546,9 +552,11 @@ static int tpm_tis_send_main(struct tpm_chip *chip, const u8 *buf, size_t len)
 		if (rc >= 0)
 			/* Data transfer done successfully */
 			break;
-		else if (rc != -EIO)
+		else if (rc != EAGAIN && rc != -EIO)
 			/* Data transfer failed, not recoverable */
 			return rc;
+
+		usleep_range(priv->timeout_min, priv->timeout_max);
 	}
 
 	/* go and do it */

@@ -1144,6 +1152,9 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
 		priv->timeout_max = TIS_TIMEOUT_MAX_ATML;
 	}
 
+	if (priv->manufacturer_id == TPM_VID_IFX)
+		set_bit(TPM_TIS_STATUS_WORKAROUND, &priv->flags);
+
 	if (is_bsw()) {
 		priv->ilb_base_addr = ioremap(INTEL_LEGACY_BLK_BASE_ADDR,
 					ILB_REMAP_SIZE);

drivers/char/tpm/tpm_tis_core.h +1 -0

index 690ad8e9b731..ce97b58dc005 100644
@@ -89,6 +89,7 @@ enum tpm_tis_flags {
 	TPM_TIS_INVALID_STATUS		= 1,
 	TPM_TIS_DEFAULT_CANCELLATION	= 2,
 	TPM_TIS_IRQ_TESTED		= 3,
+	TPM_TIS_STATUS_WORKAROUND	= 4,
 };
 
 struct tpm_tis_data {

include/linux/tpm.h +1 -0

index 20a40ade8030..6c3125300c00 100644
@@ -335,6 +335,7 @@ enum tpm2_cc_attrs {
 #define TPM_VID_WINBOND  0x1050
 #define TPM_VID_STM      0x104A
 #define TPM_VID_ATML     0x1114
+#define TPM_VID_IFX      0x15D1
 
 enum tpm_chip_flags {
 	TPM_CHIP_FLAG_BOOTSTRAPPED		= BIT(0),

Signature · 1 line

2.48.1

Jarkko Sakkinen

maintainer

2025-03-06 22:23 UTC #1 lore raw

Quoted 44 lines · Jonathan McDowell

From: Jonathan McDowell <[email protected]>
Some Infineon devices have a issue where the status register will get stuck with a quick REQUEST_USE / COMMAND_READY sequence. This is not simply a matter of requiring a longer timeout; the work around is to retry the command submission. Add appropriate logic to do this in the send path.
This is fixed in later firmware revisions, but those are not always available, and cannot generally be easily updated from outside a firmware environment.
Testing has been performed with a simple repeated loop of doing a TPM2_CC_GET_CAPABILITY for TPM_CAP_PROP_MANUFACTURER using the Go code at:
https://the.earth.li/~noodles/tpm-stuff/timeout-reproducer-simple.go
It can take several hours to reproduce, and millions of operations.
Signed-off-by Jonathan McDowell <[email protected]>
---
 drivers/char/tpm/tpm_tis_core.c | 17 ++++++++++++++---
 drivers/char/tpm/tpm_tis_core.h |  1 +
 include/linux/tpm.h             |  1 +
 3 files changed, 16 insertions(+), 3 deletions(-)
drivers/char/tpm/tpm_tis_core.c +4 -1
index 167d71747666..e4eae206a353 100644
@@ -464,7 +464,10 @@ static int tpm_tis_send_data(struct tpm_chip *chip, const u8 *buf, size_t len)
 
 		if (wait_for_tpm_stat(chip, TPM_STS_VALID, chip->timeout_c,
 					&priv->int_queue, false) < 0) {
-			rc = -ETIME;
+			if (test_bit(TPM_TIS_STATUS_WORKAROUND, &priv->flags))
+				rc = -EAGAIN;
+			else
+				rc = -ETIME;
 			goto out_err;
 		}
 		status = tpm_tis_status(chip);

drivers/char/tpm/tpm_tis_core.c static int tpm_tis_send_data(struct tpm_chip *chip, const u8 *buf, size_t len) · line 490

Quoted 8 lines

drivers/char/tpm/tpm_tis_core.c +4 -1

@@ -481,7 +484,10 @@ static int tpm_tis_send_data(struct tpm_chip *chip, const u8 *buf, size_t len)
 
 	if (wait_for_tpm_stat(chip, TPM_STS_VALID, chip->timeout_c,
 				&priv->int_queue, false) < 0) {
-		rc = -ETIME;
+		if (test_bit(TPM_TIS_STATUS_WORKAROUND, &priv->flags))
+			rc = -EAGAIN;
+		else
+			rc = -ETIME;

I'd encapsulate this inside wait_for_tpm_stat().

Quoted 31 lines

drivers/char/tpm/tpm_tis_core.c +6 -1

 		goto out_err;
 	}
 	status = tpm_tis_status(chip);

@@ -546,9 +552,11 @@ static int tpm_tis_send_main(struct tpm_chip *chip, const u8 *buf, size_t len)
 		if (rc >= 0)
 			/* Data transfer done successfully */
 			break;
-		else if (rc != -EIO)
+		else if (rc != EAGAIN && rc != -EIO)
 			/* Data transfer failed, not recoverable */
 			return rc;
+
+		usleep_range(priv->timeout_min, priv->timeout_max);
 	}
 
 	/* go and do it */

@@ -1144,6 +1152,9 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
 		priv->timeout_max = TIS_TIMEOUT_MAX_ATML;
 	}
 
+	if (priv->manufacturer_id == TPM_VID_IFX)
+		set_bit(TPM_TIS_STATUS_WORKAROUND, &priv->flags);
+
 	if (is_bsw()) {
 		priv->ilb_base_addr = ioremap(INTEL_LEGACY_BLK_BASE_ADDR,
 					ILB_REMAP_SIZE);

drivers/char/tpm/tpm_tis_core.h +0 -0

index 690ad8e9b731..ce97b58dc005 100644

drivers/char/tpm/tpm_tis_core.h enum tpm_tis_flags { · line 92

Quoted 4 lines

drivers/char/tpm/tpm_tis_core.h +1 -0

@@ -89,6 +89,7 @@ enum tpm_tis_flags {
 	TPM_TIS_INVALID_STATUS		= 1,
 	TPM_TIS_DEFAULT_CANCELLATION	= 2,
 	TPM_TIS_IRQ_TESTED		= 3,
+	TPM_TIS_STATUS_WORKAROUND	= 4,

TPM_TIS_TIMEOUT_AGAIN or maybe *_REPEAT? The current name does not tell anything.

Quoted 18 lines

drivers/char/tpm/tpm_tis_core.h +0 -0

 };
 
 struct tpm_tis_data {

include/linux/tpm.h +1 -0

index 20a40ade8030..6c3125300c00 100644
@@ -335,6 +335,7 @@ enum tpm2_cc_attrs {
 #define TPM_VID_WINBOND  0x1050
 #define TPM_VID_STM      0x104A
 #define TPM_VID_ATML     0x1114
+#define TPM_VID_IFX      0x15D1
 
 enum tpm_chip_flags {
 	TPM_CHIP_FLAG_BOOTSTRAPPED		= BIT(0),

Signature · 1 line

2.48.1

BR, Jarkko

Jonathan McDowell

author

2025-03-07 16:36 UTC #2 lore raw

Quoted 46 lines · Jarkko Sakkinen

Quoted 45 lines · Jonathan McDowell
From: Jonathan McDowell <[email protected]>
Some Infineon devices have a issue where the status register will get stuck with a quick REQUEST_USE / COMMAND_READY sequence. This is not simply a matter of requiring a longer timeout; the work around is to retry the command submission. Add appropriate logic to do this in the send path.
This is fixed in later firmware revisions, but those are not always available, and cannot generally be easily updated from outside a firmware environment.
Testing has been performed with a simple repeated loop of doing a TPM2_CC_GET_CAPABILITY for TPM_CAP_PROP_MANUFACTURER using the Go code at:
https://the.earth.li/~noodles/tpm-stuff/timeout-reproducer-simple.go
It can take several hours to reproduce, and millions of operations.
Signed-off-by Jonathan McDowell <[email protected]>
---
 drivers/char/tpm/tpm_tis_core.c | 17 ++++++++++++++---
 drivers/char/tpm/tpm_tis_core.h |  1 +
 include/linux/tpm.h             |  1 +
 3 files changed, 16 insertions(+), 3 deletions(-)
drivers/char/tpm/tpm_tis_core.c +4 -1
index 167d71747666..e4eae206a353 100644
@@ -464,7 +464,10 @@ static int tpm_tis_send_data(struct tpm_chip *chip, const u8 *buf, size_t len)
 
 		if (wait_for_tpm_stat(chip, TPM_STS_VALID, chip->timeout_c,
 					&priv->int_queue, false) < 0) {
-			rc = -ETIME;
+			if (test_bit(TPM_TIS_STATUS_WORKAROUND, &priv->flags))
+				rc = -EAGAIN;
+			else
+				rc = -ETIME;
 			goto out_err;
 		}
 		status = tpm_tis_status(chip);
@@ -481,7 +484,10 @@ static int tpm_tis_send_data(struct tpm_chip *chip, const u8 *buf, size_t len)
 

Quoted 9 lines

drivers/char/tpm/tpm_tis_core.c static int tpm_tis_send_data(struct tpm_chip *chip, const u8 *buf, size_t len) · line 470
Quoted 7 lines
drivers/char/tpm/tpm_tis_core.c +4 -1
 	if (wait_for_tpm_stat(chip, TPM_STS_VALID, chip->timeout_c,
 				&priv->int_queue, false) < 0) {
-		rc = -ETIME;
+		if (test_bit(TPM_TIS_STATUS_WORKAROUND, &priv->flags))
+			rc = -EAGAIN;
+		else
+			rc = -ETIME;
I'd encapsulate this inside wait_for_tpm_stat().

I think that gets a bit more complicated; this is an errata in the send command path, for a stuck VALID bit, and the fix is to restart the whole command send (i.e. we need to kick the TPM with tpm_tis_ready() etc). I'm not sure returning EAGAIN in wait_for_tpm_stat() then makes tpm_tis_send_data() any simpler.

Quoted 30 lines

drivers/char/tpm/tpm_tis_core.c +6 -1

 		goto out_err;
 	}
 	status = tpm_tis_status(chip);

@@ -546,9 +552,11 @@ static int tpm_tis_send_main(struct tpm_chip *chip, const u8 *buf, size_t len)
 		if (rc >= 0)
 			/* Data transfer done successfully */
 			break;
-		else if (rc != -EIO)
+		else if (rc != EAGAIN && rc != -EIO)
 			/* Data transfer failed, not recoverable */
 			return rc;
+
+		usleep_range(priv->timeout_min, priv->timeout_max);
 	}
 
 	/* go and do it */

@@ -1144,6 +1152,9 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
 		priv->timeout_max = TIS_TIMEOUT_MAX_ATML;
 	}
 
+	if (priv->manufacturer_id == TPM_VID_IFX)
+		set_bit(TPM_TIS_STATUS_WORKAROUND, &priv->flags);
+
 	if (is_bsw()) {
 		priv->ilb_base_addr = ioremap(INTEL_LEGACY_BLK_BASE_ADDR,
 					ILB_REMAP_SIZE);

drivers/char/tpm/tpm_tis_core.h +0 -0

index 690ad8e9b731..ce97b58dc005 100644

Quoted 8 lines

drivers/char/tpm/tpm_tis_core.h enum tpm_tis_flags { · line 92
Quoted 5 lines
drivers/char/tpm/tpm_tis_core.h +1 -0
@@ -89,6 +89,7 @@ enum tpm_tis_flags {
 	TPM_TIS_INVALID_STATUS		= 1,
 	TPM_TIS_DEFAULT_CANCELLATION	= 2,
 	TPM_TIS_IRQ_TESTED		= 3,
+	TPM_TIS_STATUS_WORKAROUND	= 4,
TPM_TIS_TIMEOUT_AGAIN or maybe *_REPEAT? The current name does not tell anything.

Yeah, TPM_TIS_STATUS_VALID_RETRY is perhaps clearer; it's not a timeout, and we're looking to do a retry based on STS_VALID.

Quoted 13 lines

drivers/char/tpm/tpm_tis_core.h +0 -0

 };
 
 struct tpm_tis_data {

include/linux/tpm.h +1 -0

index 20a40ade8030..6c3125300c00 100644
@@ -335,6 +335,7 @@ enum tpm2_cc_attrs {
 #define TPM_VID_WINBOND  0x1050
 #define TPM_VID_STM      0x104A
 #define TPM_VID_ATML     0x1114
+#define TPM_VID_IFX      0x15D1

Quoted 2 lines

include/linux/tpm.h enum tpm2_cc_attrs { · line 341
Quoted 2 lines
include/linux/tpm.h +0 -0
 enum tpm_chip_flags {
 	TPM_CHIP_FLAG_BOOTSTRAPPED		= BIT(0),

Signature · 3 lines

... "What's the philosophical difference between a killfile and the
    automoderation?" "A killfile throws away good posts.  Automoderation
    throws away bad posts." -- Jonathan H N Chin to Calle Dybedahl

Jarkko Sakkinen

maintainer

2025-03-07 16:46 UTC #3 lore raw

Quoted 57 lines · Jonathan McDowell

Quoted 46 lines · Jarkko Sakkinen
Quoted 45 lines · Jonathan McDowell
From: Jonathan McDowell <[email protected]>
Some Infineon devices have a issue where the status register will get stuck with a quick REQUEST_USE / COMMAND_READY sequence. This is not simply a matter of requiring a longer timeout; the work around is to retry the command submission. Add appropriate logic to do this in the send path.
This is fixed in later firmware revisions, but those are not always available, and cannot generally be easily updated from outside a firmware environment.
Testing has been performed with a simple repeated loop of doing a TPM2_CC_GET_CAPABILITY for TPM_CAP_PROP_MANUFACTURER using the Go code at:
https://the.earth.li/~noodles/tpm-stuff/timeout-reproducer-simple.go
It can take several hours to reproduce, and millions of operations.
Signed-off-by Jonathan McDowell <[email protected]>
---
 drivers/char/tpm/tpm_tis_core.c | 17 ++++++++++++++---
 drivers/char/tpm/tpm_tis_core.h |  1 +
 include/linux/tpm.h             |  1 +
 3 files changed, 16 insertions(+), 3 deletions(-)
drivers/char/tpm/tpm_tis_core.c +4 -1
index 167d71747666..e4eae206a353 100644
@@ -464,7 +464,10 @@ static int tpm_tis_send_data(struct tpm_chip *chip, const u8 *buf, size_t len)
 
 		if (wait_for_tpm_stat(chip, TPM_STS_VALID, chip->timeout_c,
 					&priv->int_queue, false) < 0) {
-			rc = -ETIME;
+			if (test_bit(TPM_TIS_STATUS_WORKAROUND, &priv->flags))
+				rc = -EAGAIN;
+			else
+				rc = -ETIME;
 			goto out_err;
 		}
 		status = tpm_tis_status(chip);
@@ -481,7 +484,10 @@ static int tpm_tis_send_data(struct tpm_chip *chip, const u8 *buf, size_t len)
 
Quoted 9 lines
drivers/char/tpm/tpm_tis_core.c static int tpm_tis_send_data(struct tpm_chip *chip, const u8 *buf, size_t len) · line 470
Quoted 7 lines
drivers/char/tpm/tpm_tis_core.c +4 -1
 	if (wait_for_tpm_stat(chip, TPM_STS_VALID, chip->timeout_c,
 				&priv->int_queue, false) < 0) {
-		rc = -ETIME;
+		if (test_bit(TPM_TIS_STATUS_WORKAROUND, &priv->flags))
+			rc = -EAGAIN;
+		else
+			rc = -ETIME;
I'd encapsulate this inside wait_for_tpm_stat().

Quoted 5 lines

I think that gets a bit more complicated; this is an errata in the send command path, for a stuck VALID bit, and the fix is to restart the whole command send (i.e. we need to kick the TPM with tpm_tis_ready() etc). I'm not sure returning EAGAIN in wait_for_tpm_stat() then makes tpm_tis_send_data() any simpler.

OK, it is a fair argument. Let's keep it as it is.

Quoted 40 lines

Quoted 31 lines

drivers/char/tpm/tpm_tis_core.c +6 -1

 		goto out_err;
 	}
 	status = tpm_tis_status(chip);

@@ -546,9 +552,11 @@ static int tpm_tis_send_main(struct tpm_chip *chip, const u8 *buf, size_t len)
 		if (rc >= 0)
 			/* Data transfer done successfully */
 			break;
-		else if (rc != -EIO)
+		else if (rc != EAGAIN && rc != -EIO)
 			/* Data transfer failed, not recoverable */
 			return rc;
+
+		usleep_range(priv->timeout_min, priv->timeout_max);
 	}
 
 	/* go and do it */

@@ -1144,6 +1152,9 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
 		priv->timeout_max = TIS_TIMEOUT_MAX_ATML;
 	}
 
+	if (priv->manufacturer_id == TPM_VID_IFX)
+		set_bit(TPM_TIS_STATUS_WORKAROUND, &priv->flags);
+
 	if (is_bsw()) {
 		priv->ilb_base_addr = ioremap(INTEL_LEGACY_BLK_BASE_ADDR,
 					ILB_REMAP_SIZE);

drivers/char/tpm/tpm_tis_core.h +0 -0

index 690ad8e9b731..ce97b58dc005 100644

Quoted 8 lines

drivers/char/tpm/tpm_tis_core.h enum tpm_tis_flags { · line 92
Quoted 5 lines
drivers/char/tpm/tpm_tis_core.h +1 -0
@@ -89,6 +89,7 @@ enum tpm_tis_flags {
 	TPM_TIS_INVALID_STATUS		= 1,
 	TPM_TIS_DEFAULT_CANCELLATION	= 2,
 	TPM_TIS_IRQ_TESTED		= 3,
+	TPM_TIS_STATUS_WORKAROUND	= 4,
TPM_TIS_TIMEOUT_AGAIN or maybe *_REPEAT? The current name does not tell anything.

Quoted 2 lines

Yeah, TPM_TIS_STATUS_VALID_RETRY is perhaps clearer; it's not a timeout, and we're looking to do a retry based on STS_VALID.

WFM

Quoted 14 lines

drivers/char/tpm/tpm_tis_core.h +0 -0

 };
 
 struct tpm_tis_data {

include/linux/tpm.h +1 -0

index 20a40ade8030..6c3125300c00 100644
@@ -335,6 +335,7 @@ enum tpm2_cc_attrs {
 #define TPM_VID_WINBOND  0x1050
 #define TPM_VID_STM      0x104A
 #define TPM_VID_ATML     0x1114
+#define TPM_VID_IFX      0x15D1

Quoted 9 lines

Quoted 2 lines

include/linux/tpm.h enum tpm2_cc_attrs { · line 341
Quoted 2 lines
include/linux/tpm.h +0 -0
 enum tpm_chip_flags {
 	TPM_CHIP_FLAG_BOOTSTRAPPED		= BIT(0),

Signature · 3 lines

... "What's the philosophical difference between a killfile and the
    automoderation?" "A killfile throws away good posts.  Automoderation
    throws away bad posts." -- Jonathan H N Chin to Calle Dybedahl

BR, Jarkko

Jonathan McDowell

author

2025-03-10 12:20 UTC #4 lore raw

PATCH v2 1/1 tpm, tpm_tis: Workaround failed command reception on Infineon devices

From: Jonathan McDowell <[email protected]>

This is fixed in later firmware revisions, but those are not always available, and cannot generally be easily updated from outside a firmware environment.

Testing has been performed with a simple repeated loop of doing a TPM2_CC_GET_CAPABILITY for TPM_CAP_PROP_MANUFACTURER using the Go code at:

https://the.earth.li/~noodles/tpm-stuff/timeout-reproducer-simple.go

It can take several hours to reproduce, and several million operations.

Signed-off-by Jonathan McDowell <[email protected]>

---
v2: Rename flag to TPM_TIS_STATUS_VALID_RETRY

 drivers/char/tpm/tpm_tis_core.c | 17 ++++++++++++++---
 drivers/char/tpm/tpm_tis_core.h |  1 +
 include/linux/tpm.h             |  1 +
 3 files changed, 16 insertions(+), 3 deletions(-)

drivers/char/tpm/tpm_tis_core.c +14 -3

index c969a1793184..4ab69c3e103c 100644
@@ -463,7 +463,10 @@ static int tpm_tis_send_data(struct tpm_chip *chip, const u8 *buf, size_t len)
 
 		if (wait_for_tpm_stat(chip, TPM_STS_VALID, chip->timeout_c,
 					&priv->int_queue, false) < 0) {
-			rc = -ETIME;
+			if (test_bit(TPM_TIS_STATUS_VALID_RETRY, &priv->flags))
+				rc = -EAGAIN;
+			else
+				rc = -ETIME;
 			goto out_err;
 		}
 		status = tpm_tis_status(chip);

@@ -480,7 +483,10 @@ static int tpm_tis_send_data(struct tpm_chip *chip, const u8 *buf, size_t len)
 
 	if (wait_for_tpm_stat(chip, TPM_STS_VALID, chip->timeout_c,
 				&priv->int_queue, false) < 0) {
-		rc = -ETIME;
+		if (test_bit(TPM_TIS_STATUS_VALID_RETRY, &priv->flags))
+			rc = -EAGAIN;
+		else
+			rc = -ETIME;
 		goto out_err;
 	}
 	status = tpm_tis_status(chip);

@@ -545,9 +551,11 @@ static int tpm_tis_send_main(struct tpm_chip *chip, const u8 *buf, size_t len)
 		if (rc >= 0)
 			/* Data transfer done successfully */
 			break;
-		else if (rc != -EIO)
+		else if (rc != EAGAIN && rc != -EIO)
 			/* Data transfer failed, not recoverable */
 			return rc;
+
+		usleep_range(priv->timeout_min, priv->timeout_max);
 	}
 
 	/* go and do it */

@@ -1143,6 +1151,9 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
 		priv->timeout_max = TIS_TIMEOUT_MAX_ATML;
 	}
 
+	if (priv->manufacturer_id == TPM_VID_IFX)
+		set_bit(TPM_TIS_STATUS_VALID_RETRY, &priv->flags);
+
 	if (is_bsw()) {
 		priv->ilb_base_addr = ioremap(INTEL_LEGACY_BLK_BASE_ADDR,
 					ILB_REMAP_SIZE);

drivers/char/tpm/tpm_tis_core.h +1 -0

index 690ad8e9b731..970d02c337c7 100644
@@ -89,6 +89,7 @@ enum tpm_tis_flags {
 	TPM_TIS_INVALID_STATUS		= 1,
 	TPM_TIS_DEFAULT_CANCELLATION	= 2,
 	TPM_TIS_IRQ_TESTED		= 3,
+	TPM_TIS_STATUS_VALID_RETRY	= 4,
 };
 
 struct tpm_tis_data {

include/linux/tpm.h +1 -0

index 20a40ade8030..6c3125300c00 100644
@@ -335,6 +335,7 @@ enum tpm2_cc_attrs {
 #define TPM_VID_WINBOND  0x1050
 #define TPM_VID_STM      0x104A
 #define TPM_VID_ATML     0x1114
+#define TPM_VID_IFX      0x15D1
 
 enum tpm_chip_flags {
 	TPM_CHIP_FLAG_BOOTSTRAPPED		= BIT(0),

Signature · 1 line

2.48.1

Paul Menzel

2025-03-10 14:12 UTC #5 lore raw

Dear Jonathan,

Some nits, should you resend. Feel free to ignore. The verb *work around* is spelled with a space.

Am 10.03.25 um 13:19 schrieb Jonathan McDowell:

Quoted 5 lines

From: Jonathan McDowell <[email protected]>
Some Infineon devices have a issue where the status register will get stuck with a quick REQUEST_USE / COMMAND_READY sequence. This is not simply a matter of requiring a longer timeout; the work around is to

The noun without. ;-)

Quoted 2 lines

retry the command submission. Add appropriate logic to do this in the send path.

Does the workaround have downsides?

Quoted 3 lines

This is fixed in later firmware revisions, but those are not always available, and cannot generally be easily updated from outside a firmware environment.

Please mention the affected revisions. Is there an errata for tis.

Quoted 5 lines

Testing has been performed with a simple repeated loop of doing a TPM2_CC_GET_CAPABILITY for TPM_CAP_PROP_MANUFACTURER using the Go code at:
https://the.earth.li/~noodles/tpm-stuff/timeout-reproducer-simple.go

Awesome. Thank you for sharing.

Quoted 16 lines

It can take several hours to reproduce, and several million operations.
Signed-off-by Jonathan McDowell <[email protected]>
---
v2: Rename flag to TPM_TIS_STATUS_VALID_RETRY
  drivers/char/tpm/tpm_tis_core.c | 17 ++++++++++++++---
  drivers/char/tpm/tpm_tis_core.h |  1 +
  include/linux/tpm.h             |  1 +
  3 files changed, 16 insertions(+), 3 deletions(-)
drivers/char/tpm/tpm_tis_core.c +0 -0
index c969a1793184..4ab69c3e103c 100644

drivers/char/tpm/tpm_tis_core.c static int tpm_tis_send_data(struct tpm_chip *chip, · line 470

Quoted 9 lines

drivers/char/tpm/tpm_tis_core.c +4 -1

@@ -463,7 +463,10 @@ static int tpm_tis_send_data(struct tpm_chip *chip, 
 const u8 *buf, size_t len)
 
          if (wait_for_tpm_stat(chip, TPM_STS_VALID, chip->timeout_c,
                      &priv->int_queue, false) < 0) {
-            rc = -ETIME;
+            if (test_bit(TPM_TIS_STATUS_VALID_RETRY, &priv->flags))
+                rc = -EAGAIN;
+            else
+                rc = -ETIME;

I’d use a ternary operator as the same variable is assigned to.

Quoted 65 lines

drivers/char/tpm/tpm_tis_core.c +0 -0

              goto out_err;
          }

status = tpm_tis_status(chip);

drivers/char/tpm/tpm_tis_core.c +4 -1

@@ -480,7 +483,10 @@ static int tpm_tis_send_data(struct tpm_chip *chip, 
 const u8 *buf, size_t len)
 
      if (wait_for_tpm_stat(chip, TPM_STS_VALID, chip->timeout_c,
                  &priv->int_queue, false) < 0) {
-        rc = -ETIME;
+        if (test_bit(TPM_TIS_STATUS_VALID_RETRY, &priv->flags))
+            rc = -EAGAIN;
+        else
+            rc = -ETIME;
          goto out_err;
      }

status = tpm_tis_status(chip);

drivers/char/tpm/tpm_tis_core.c +3 -1

@@ -545,9 +551,11 @@ static int tpm_tis_send_main(struct tpm_chip *chip, 
 const u8 *buf, size_t len)
          if (rc >= 0)
              /* Data transfer done successfully */
              break;
-        else if (rc != -EIO)
+        else if (rc != EAGAIN && rc != -EIO)
              /* Data transfer failed, not recoverable */
              return rc;
+
+        usleep_range(priv->timeout_min, priv->timeout_max);
      }

/* go and do it */

drivers/char/tpm/tpm_tis_core.c +3 -0

@@ -1143,6 +1151,9 @@ int tpm_tis_core_init(struct device *dev, struct 
 tpm_tis_data *priv, int irq,
          priv->timeout_max = TIS_TIMEOUT_MAX_ATML;
      }
 
+    if (priv->manufacturer_id == TPM_VID_IFX)
+        set_bit(TPM_TIS_STATUS_VALID_RETRY, &priv->flags);
+
      if (is_bsw()) {
          priv->ilb_base_addr = ioremap(INTEL_LEGACY_BLK_BASE_ADDR,

ILB_REMAP_SIZE);

drivers/char/tpm/tpm_tis_core.h +1 -0

index 690ad8e9b731..970d02c337c7 100644
@@ -89,6 +89,7 @@ enum tpm_tis_flags {
      TPM_TIS_INVALID_STATUS        = 1,
      TPM_TIS_DEFAULT_CANCELLATION    = 2,
      TPM_TIS_IRQ_TESTED        = 3,
+    TPM_TIS_STATUS_VALID_RETRY    = 4,
  };
 
  struct tpm_tis_data {

include/linux/tpm.h +1 -0

index 20a40ade8030..6c3125300c00 100644
@@ -335,6 +335,7 @@ enum tpm2_cc_attrs {
  #define TPM_VID_WINBOND  0x1050
  #define TPM_VID_STM      0x104A
  #define TPM_VID_ATML     0x1114
+#define TPM_VID_IFX      0x15D1
 
  enum tpm_chip_flags {
      TPM_CHIP_FLAG_BOOTSTRAPPED        = BIT(0),

Jarkko Sakkinen

maintainer Reviewed-by on v2 1/1

2025-03-11 09:46 UTC #6 lore raw

In reply to Jonathan McDowell

Quoted 101 lines · Jonathan McDowell

From: Jonathan McDowell <[email protected]>

This is fixed in later firmware revisions, but those are not always available, and cannot generally be easily updated from outside a firmware environment.

Testing has been performed with a simple repeated loop of doing a TPM2_CC_GET_CAPABILITY for TPM_CAP_PROP_MANUFACTURER using the Go code at:

https://the.earth.li/~noodles/tpm-stuff/timeout-reproducer-simple.go

It can take several hours to reproduce, and several million operations.

Signed-off-by Jonathan McDowell <[email protected]>

---
v2: Rename flag to TPM_TIS_STATUS_VALID_RETRY

 drivers/char/tpm/tpm_tis_core.c | 17 ++++++++++++++---
 drivers/char/tpm/tpm_tis_core.h |  1 +
 include/linux/tpm.h             |  1 +
 3 files changed, 16 insertions(+), 3 deletions(-)

drivers/char/tpm/tpm_tis_core.c +14 -3

index c969a1793184..4ab69c3e103c 100644
@@ -463,7 +463,10 @@ static int tpm_tis_send_data(struct tpm_chip *chip, const u8 *buf, size_t len)
 		if (wait_for_tpm_stat(chip, TPM_STS_VALID, chip->timeout_c,
 					&priv->int_queue, false) < 0) {
-			rc = -ETIME;
+			if (test_bit(TPM_TIS_STATUS_VALID_RETRY, &priv->flags))
+				rc = -EAGAIN;
+			else
+				rc = -ETIME;
 			goto out_err;
 		}
 		status = tpm_tis_status(chip);

@@ -480,7 +483,10 @@ static int tpm_tis_send_data(struct tpm_chip *chip, const u8 *buf, size_t len)
 	if (wait_for_tpm_stat(chip, TPM_STS_VALID, chip->timeout_c,
 				&priv->int_queue, false) < 0) {
-		rc = -ETIME;
+		if (test_bit(TPM_TIS_STATUS_VALID_RETRY, &priv->flags))
+			rc = -EAGAIN;
+		else
+			rc = -ETIME;
 		goto out_err;
 	}
 	status = tpm_tis_status(chip);

@@ -545,9 +551,11 @@ static int tpm_tis_send_main(struct tpm_chip *chip, const u8 *buf, size_t len)
 		if (rc >= 0)
 			/* Data transfer done successfully */
 			break;
-		else if (rc != -EIO)
+		else if (rc != EAGAIN && rc != -EIO)
 			/* Data transfer failed, not recoverable */
 			return rc;
+
+		usleep_range(priv->timeout_min, priv->timeout_max);
 	}
 	/* go and do it */

@@ -1143,6 +1151,9 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
 		priv->timeout_max = TIS_TIMEOUT_MAX_ATML;
 	}
+	if (priv->manufacturer_id == TPM_VID_IFX)
+		set_bit(TPM_TIS_STATUS_VALID_RETRY, &priv->flags);
+
 	if (is_bsw()) {
 		priv->ilb_base_addr = ioremap(INTEL_LEGACY_BLK_BASE_ADDR,
 					ILB_REMAP_SIZE);

drivers/char/tpm/tpm_tis_core.h +1 -0

index 690ad8e9b731..970d02c337c7 100644
@@ -89,6 +89,7 @@ enum tpm_tis_flags {
 	TPM_TIS_INVALID_STATUS		= 1,
 	TPM_TIS_DEFAULT_CANCELLATION	= 2,
 	TPM_TIS_IRQ_TESTED		= 3,
+	TPM_TIS_STATUS_VALID_RETRY	= 4,
 };
 struct tpm_tis_data {

include/linux/tpm.h +1 -0

index 20a40ade8030..6c3125300c00 100644
@@ -335,6 +335,7 @@ enum tpm2_cc_attrs {
 #define TPM_VID_WINBOND  0x1050
 #define TPM_VID_STM      0x104A
 #define TPM_VID_ATML     0x1114
+#define TPM_VID_IFX      0x15D1
 enum tpm_chip_flags {
 	TPM_CHIP_FLAG_BOOTSTRAPPED		= BIT(0),

Signature · 1 line

2.48.1

Reviewed-by Jarkko Sakkinen <[email protected]>

BR, Jarkko

Jonathan McDowell

author

2025-03-21 16:49 UTC #7 lore raw

In reply to Jonathan McDowell

Jarkko, I've realised I've somehow introduced a typo in the patch below that means it doesn't fire correctly; I'm not sure how this happened as my local copy I was testing on is definitely correct. Would you like a one line fix up patch, or can you manually fix it up in your tree?

This hunk:

drivers/char/tpm/tpm_tis_core.c static int tpm_tis_send_main(struct tpm_chip *chip, const u8 *buf, size_t len) · line 560

Quoted 12 lines

drivers/char/tpm/tpm_tis_core.c +3 -1

@@ -545,9 +551,11 @@ static int tpm_tis_send_main(struct tpm_chip *chip, const u8 *buf, size_t len)
 		if (rc >= 0)
 			/* Data transfer done successfully */
 			break;
-		else if (rc != -EIO)
+		else if (rc != EAGAIN && rc != -EIO)
 			/* Data transfer failed, not recoverable */
 			return rc;
+
+		usleep_range(priv->timeout_min, priv->timeout_max);
 	}
 	/* go and do it */

should be "rc != -EAGAIN" - the "-" sign has somehow been lost.

Apologies for this, let me know what's easiest for you in terms of resolving it.

Quoted 102 lines · Jonathan McDowell

From: Jonathan McDowell <[email protected]>

This is fixed in later firmware revisions, but those are not always available, and cannot generally be easily updated from outside a firmware environment.

Testing has been performed with a simple repeated loop of doing a TPM2_CC_GET_CAPABILITY for TPM_CAP_PROP_MANUFACTURER using the Go code at:

https://the.earth.li/~noodles/tpm-stuff/timeout-reproducer-simple.go

It can take several hours to reproduce, and several million operations.

Signed-off-by Jonathan McDowell <[email protected]>

---
v2: Rename flag to TPM_TIS_STATUS_VALID_RETRY

drivers/char/tpm/tpm_tis_core.c | 17 ++++++++++++++---
drivers/char/tpm/tpm_tis_core.h |  1 +
include/linux/tpm.h             |  1 +
3 files changed, 16 insertions(+), 3 deletions(-)

drivers/char/tpm/tpm_tis_core.c +14 -3

index c969a1793184..4ab69c3e103c 100644
@@ -463,7 +463,10 @@ static int tpm_tis_send_data(struct tpm_chip *chip, const u8 *buf, size_t len)
 		if (wait_for_tpm_stat(chip, TPM_STS_VALID, chip->timeout_c,
 					&priv->int_queue, false) < 0) {
-			rc = -ETIME;
+			if (test_bit(TPM_TIS_STATUS_VALID_RETRY, &priv->flags))
+				rc = -EAGAIN;
+			else
+				rc = -ETIME;
 			goto out_err;
 		}
 		status = tpm_tis_status(chip);

@@ -480,7 +483,10 @@ static int tpm_tis_send_data(struct tpm_chip *chip, const u8 *buf, size_t len)
 	if (wait_for_tpm_stat(chip, TPM_STS_VALID, chip->timeout_c,
 				&priv->int_queue, false) < 0) {
-		rc = -ETIME;
+		if (test_bit(TPM_TIS_STATUS_VALID_RETRY, &priv->flags))
+			rc = -EAGAIN;
+		else
+			rc = -ETIME;
 		goto out_err;
 	}
 	status = tpm_tis_status(chip);

@@ -545,9 +551,11 @@ static int tpm_tis_send_main(struct tpm_chip *chip, const u8 *buf, size_t len)
 		if (rc >= 0)
 			/* Data transfer done successfully */
 			break;
-		else if (rc != -EIO)
+		else if (rc != EAGAIN && rc != -EIO)
 			/* Data transfer failed, not recoverable */
 			return rc;
+
+		usleep_range(priv->timeout_min, priv->timeout_max);
 	}
 	/* go and do it */

@@ -1143,6 +1151,9 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
 		priv->timeout_max = TIS_TIMEOUT_MAX_ATML;
 	}
+	if (priv->manufacturer_id == TPM_VID_IFX)
+		set_bit(TPM_TIS_STATUS_VALID_RETRY, &priv->flags);
+
 	if (is_bsw()) {
 		priv->ilb_base_addr = ioremap(INTEL_LEGACY_BLK_BASE_ADDR,
 					ILB_REMAP_SIZE);

drivers/char/tpm/tpm_tis_core.h +1 -0

index 690ad8e9b731..970d02c337c7 100644
@@ -89,6 +89,7 @@ enum tpm_tis_flags {
 	TPM_TIS_INVALID_STATUS		= 1,
 	TPM_TIS_DEFAULT_CANCELLATION	= 2,
 	TPM_TIS_IRQ_TESTED		= 3,
+	TPM_TIS_STATUS_VALID_RETRY	= 4,
 };
 struct tpm_tis_data {

include/linux/tpm.h +1 -0

index 20a40ade8030..6c3125300c00 100644
@@ -335,6 +335,7 @@ enum tpm2_cc_attrs {
 #define TPM_VID_WINBOND  0x1050
 #define TPM_VID_STM      0x104A
 #define TPM_VID_ATML     0x1114
+#define TPM_VID_IFX      0x15D1
 enum tpm_chip_flags {
 	TPM_CHIP_FLAG_BOOTSTRAPPED		= BIT(0),

Signature · 1 line

2.48.1

Signature · 1 line

101 things you can't have too much of : 7 - Memory.

Jarkko Sakkinen

maintainer

2025-03-22 21:10 UTC #8 lore raw

Quoted 22 lines · Jonathan McDowell

Jarkko, I've realised I've somehow introduced a typo in the patch below that means it doesn't fire correctly; I'm not sure how this happened as my local copy I was testing on is definitely correct. Would you like a one line fix up patch, or can you manually fix it up in your tree?
This hunk:
Review static int tpm_tis_send_main(struct tpm_chip *chip, const u8 *buf, size_t len) · line 560
Quoted 12 lines
(quoted diff) +3 -1
@@ -545,9 +551,11 @@ static int tpm_tis_send_main(struct tpm_chip *chip, const u8 *buf, size_t len)
 		if (rc >= 0)
 			/* Data transfer done successfully */
 			break;
-		else if (rc != -EIO)
+		else if (rc != EAGAIN && rc != -EIO)
 			/* Data transfer failed, not recoverable */
 			return rc;
+
+		usleep_range(priv->timeout_min, priv->timeout_max);
 	}
 	/* go and do it */
should be "rc != -EAGAIN" - the "-" sign has somehow been lost.

Quoted 2 lines

Apologies for this, let me know what's easiest for you in terms of resolving it.

NP, I missed it too so we're in the same boat ;-)

I did:

$ git -P diff

drivers/char/tpm/tpm_tis_core.c +1 -1

index 4ab69c3e103c..ed0d3d8449b3 100644
@@ -551,7 +551,7 @@ static int tpm_tis_send_main(struct tpm_chip *chip, const u8 *buf, size_t len)
 		if (rc >= 0)
 			/* Data transfer done successfully */
 			break;
-		else if (rc != EAGAIN && rc != -EIO)
+		else if (rc != -EAGAIN && rc != -EIO)
 			/* Data transfer failed, not recoverable */
 			return rc;

Ping, if anything else.

BR, Jarkko