diff mbox series

[v2,3/4] tpm: Implement command and response retry in tpm_tis_core

Message ID 20220506170013.22598-3-johannes.holland@infineon.com (mailing list archive)
State New, archived
Headers show
Series [v2,1/4] tpm: Add tpm_tis_i2c backend for tpm_tis_core | expand

Commit Message

Johannes Holland May 6, 2022, 5 p.m. UTC 
Some errors during command transmission and response reception are
recoverable. Implement the specified retry mechanisms.

Recoverable errors during response reception:
 * invalid response size during header read
 * left over data:
   a communication error can lead to a FIFO read of 0xFFs and an
   unexpected STS.dataAvail = 1, subsequently
 * CRC mismatch

Recoverable errors during transmit:
 * CRC mismatch

Signed-off-by: Johannes Holland <johannes.holland@infineon.com>
---
 drivers/char/tpm/tpm_tis_core.c | 98 +++++++++++++++++++--------------
 drivers/char/tpm/tpm_tis_core.h |  1 +
 2 files changed, 57 insertions(+), 42 deletions(-)

Comments

Jarkko Sakkinen May 7, 2022, 7:56 p.m. UTC | #1 
On Fri, May 06, 2022 at 07:00:15PM +0200, Johannes Holland wrote:
> Some errors during command transmission and response reception are
> recoverable. Implement the specified retry mechanisms.
> 
> Recoverable errors during response reception:
>  * invalid response size during header read
>  * left over data:
>    a communication error can lead to a FIFO read of 0xFFs and an
>    unexpected STS.dataAvail = 1, subsequently
>  * CRC mismatch
> 
> Recoverable errors during transmit:
>  * CRC mismatch
> 
> Signed-off-by: Johannes Holland <johannes.holland@infineon.com>

You should split this multiple patch, each implementing one retry
mechanism (CRC's can for recv and transmit can be in a single path
tho):

https://www.kernel.org/doc/html/latest/process/submitting-patches.html#separate-your-changes

BR, Jarkko
diff mbox series

Patch

diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index f1c893a5a38f..a2b6fba7f719 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -287,6 +287,7 @@  static int tpm_tis_recv(struct tpm_chip *chip, u8 *buf, size_t count)
 {
 	struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
 	int size = 0;
+	int i = 0;
 	int status;
 	u32 expected;
 	int rc;
@@ -296,45 +297,52 @@  static int tpm_tis_recv(struct tpm_chip *chip, u8 *buf, size_t count)
 		goto out;
 	}
 
-	size = recv_data(chip, buf, TPM_HEADER_SIZE);
-	/* read first 10 bytes, including tag, paramsize, and result */
-	if (size < TPM_HEADER_SIZE) {
-		dev_err(&chip->dev, "Unable to read header\n");
-		goto out;
-	}
+	do {
+		if (size < 0)
+			tpm_tis_write8(priv, TPM_STS(priv->locality),
+				       TPM_STS_RESPONSE_RETRY);
+
+		size = recv_data(chip, buf, TPM_HEADER_SIZE);
+		/* read first 10 bytes, including tag, paramsize, and result */
+		if (size < TPM_HEADER_SIZE) {
+			dev_err(&chip->dev, "Unable to read header\n");
+			goto out;
+		}
 
-	expected = be32_to_cpu(*(__be32 *) (buf + 2));
-	if (expected > count || expected < TPM_HEADER_SIZE) {
-		size = -EIO;
-		goto out;
-	}
+		expected = be32_to_cpu(*(__be32 *)(buf + 2));
+		if (expected > count || expected < TPM_HEADER_SIZE) {
+			dev_info(&chip->dev, "Bad response size: %d. Retry...\n", expected);
+			size = -EIO;
+			continue;
+		}
 
-	size += recv_data(chip, &buf[TPM_HEADER_SIZE],
-			  expected - TPM_HEADER_SIZE);
-	if (size < expected) {
-		dev_err(&chip->dev, "Unable to read remainder of result\n");
-		size = -ETIME;
-		goto out;
-	}
+		size += recv_data(chip, &buf[TPM_HEADER_SIZE],
+				expected - TPM_HEADER_SIZE);
+		if (size < expected) {
+			dev_err(&chip->dev, "Unable to read remainder of result\n");
+			size = -ETIME;
+			goto out;
+		}
 
-	if (wait_for_tpm_stat(chip, TPM_STS_VALID, chip->timeout_c,
-				&priv->int_queue, false) < 0) {
-		size = -ETIME;
-		goto out;
-	}
-	status = tpm_tis_status(chip);
-	if (status & TPM_STS_DATA_AVAIL) {	/* retry? */
-		dev_err(&chip->dev, "Error left over data\n");
-		size = -EIO;
-		goto out;
-	}
+		if (wait_for_tpm_stat(chip, TPM_STS_VALID, chip->timeout_c,
+				      &priv->int_queue, false) < 0) {
+			size = -ETIME;
+			goto out;
+		}
+		status = tpm_tis_status(chip);
+		if (status & TPM_STS_DATA_AVAIL) {
+			dev_info(&chip->dev, "Error left over data. Retry...\n");
+			size = -EIO;
+			continue;
+		}
 
-	rc = tpm_tis_verify_crc(priv, (size_t)size, buf);
-	if (rc < 0) {
-		dev_err(&chip->dev, "Error crc mismatch for response.\n");
-		size = rc;
-		goto out;
-	}
+		rc = tpm_tis_verify_crc(priv, (size_t)size, buf);
+		if (rc < 0) {
+			dev_info(&chip->dev, "Error crc mismatch for response. Retry...\n");
+			size = rc;
+			continue;
+		}
+	} while (unlikely(size < 0) && i++ < TPM_RETRY);
 
 out:
 	tpm_tis_ready(chip);
@@ -444,18 +452,24 @@  static int tpm_tis_send_main(struct tpm_chip *chip, const u8 *buf, size_t len)
 {
 	struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
 	int rc;
+	int i = 0;
 	u32 ordinal;
 	unsigned long dur;
 
-	rc = tpm_tis_send_data(chip, buf, len);
-	if (rc < 0)
-		return rc;
+	do {
+		rc = tpm_tis_send_data(chip, buf, len);
+		if (rc < 0)
+			return rc;
 
-	rc = tpm_tis_verify_crc(priv, len, buf);
-	if (rc < 0) {
-		dev_err(&chip->dev, "Error crc mismatch for command.\n");
+		rc = tpm_tis_verify_crc(priv, len, buf);
+		if (rc < 0) {
+			dev_info(&chip->dev, "Error crc mismatch for command. Retry...\n");
+			tpm_tis_ready(chip);
+		}
+	} while (unlikely(rc < 0) && i++ < TPM_RETRY);
+
+	if (rc < 0)
 		return rc;
-	}
 
 	/* go and do it */
 	rc = tpm_tis_write8(priv, TPM_STS(priv->locality), TPM_STS_GO);
diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h
index 66a5a13cd1df..d3d7c45cb762 100644
--- a/drivers/char/tpm/tpm_tis_core.h
+++ b/drivers/char/tpm/tpm_tis_core.h
@@ -34,6 +34,7 @@  enum tis_status {
 	TPM_STS_GO = 0x20,
 	TPM_STS_DATA_AVAIL = 0x10,
 	TPM_STS_DATA_EXPECT = 0x08,
+	TPM_STS_RESPONSE_RETRY = 0x02,
 	TPM_STS_READ_ZERO = 0x23, /* bits that must be zero on read */
 };