All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexander Steffen <Alexander.Steffen@infineon.com>
To: <jarkko.sakkinen@linux.intel.com>, <nayna@linux.vnet.ibm.com>,
	<kgold@linux.vnet.ibm.com>, <linux-integrity@vger.kernel.org>
Cc: Alexander Steffen <Alexander.Steffen@infineon.com>
Subject: [RFC][PATCH 4/9] tpm_tis_core: send all data in single operation
Date: Fri, 8 Dec 2017 19:46:53 +0100	[thread overview]
Message-ID: <20171208184658.9588-5-Alexander.Steffen@infineon.com> (raw)
In-Reply-To: <20171208184658.9588-1-Alexander.Steffen@infineon.com>

tpm_tis_send_data splits all commands sent to the TPM into at least two
transfers, even if the commands are small enough to fit into a single
transfer. The intention seems to be to make extra sure that the TPM has
received all data correctly by observing the Expect flag flipping from 1 to
0 when writing the last byte.

Unfortunately, this does not work as intended, because the Expect flag will
change not when the last byte is written to the FIFO, but when the TPM
processes the last byte from the FIFO. With a large FIFO and long commands
it might well be that there are still many bytes left in the FIFO when the
Expect flag is checked. Obviously, the flag will be 1 then, even if the
FIFO contains more bytes than expected. Since there is no indication
whether the FIFO is already empty or not, there is no way to implement this
check reliably.

But then again, this check is not necessary at all. The driver already
ensures that not more data is sent to the TPM than is announced in the
header. In addition, the TPM is required to throw away all extraneous bytes
anyway. So if the Expect flag is 0 after all bytes have been sent to the
TPM, the TPM has received the command correctly and is ready to execute it.

Therefore, remove this intermediary check and send all data in a single
operation. This simplifies the code and improves the performance,
especially for short commands.

Signed-off-by: Alexander Steffen <Alexander.Steffen@infineon.com>
---
 drivers/char/tpm/tpm_tis_core.c | 25 ++++++-------------------
 1 file changed, 6 insertions(+), 19 deletions(-)

diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index 0df05b4..1359c52 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -320,7 +320,6 @@ static int tpm_tis_send_data(struct tpm_chip *chip, const u8 *buf, size_t len)
 	struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
 	int rc, status, burstcnt;
 	size_t count = 0;
-	bool itpm = priv->flags & TPM_TIS_ITPM_WORKAROUND;
 
 	status = tpm_tis_status(chip);
 	if ((status & TPM_STS_COMMAND_READY) == 0) {
@@ -333,38 +332,26 @@ static int tpm_tis_send_data(struct tpm_chip *chip, const u8 *buf, size_t len)
 		}
 	}
 
-	while (count < len - 1) {
+	while (count < len) {
 		burstcnt = get_burstcount(chip);
 		if (burstcnt < 0) {
 			dev_err(&chip->dev, "Unable to read burstcount\n");
 			rc = burstcnt;
 			goto out_err;
 		}
-		burstcnt = min_t(int, burstcnt, len - count - 1);
+		burstcnt = min_t(int, burstcnt, len - count);
 		rc = tpm_tis_write_bytes(priv, TPM_DATA_FIFO(priv->locality),
 					 burstcnt, buf + count);
 		if (rc < 0)
 			goto out_err;
 
 		count += burstcnt;
-
-		if (!itpm && wait_for_tpm_stat
-			     (chip, TPM_STS_DATA_EXPECT | TPM_STS_VALID,
-			      TPM_STS_DATA_EXPECT | TPM_STS_VALID,
-			      chip->timeout_c, &priv->int_queue, false) < 0) {
-			rc = -EIO;
-			goto out_err;
-		}
 	}
 
-	/* write last byte */
-	rc = tpm_tis_write8(priv, TPM_DATA_FIFO(priv->locality), buf[count]);
-	if (rc < 0)
-		goto out_err;
-
-	if (!itpm && wait_for_tpm_stat
-		     (chip, TPM_STS_DATA_EXPECT | TPM_STS_VALID, TPM_STS_VALID,
-		      chip->timeout_c, &priv->int_queue, false) < 0) {
+	if (!(priv->flags & TPM_TIS_ITPM_WORKAROUND) &&
+	    wait_for_tpm_stat(chip, TPM_STS_DATA_EXPECT | TPM_STS_VALID,
+			      TPM_STS_VALID, chip->timeout_c, &priv->int_queue,
+			      false) < 0) {
 		rc = -EIO;
 		goto out_err;
 	}
-- 
2.7.4

  parent reply	other threads:[~2017-12-08 18:50 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-08 18:46 [RFC][PATCH 0/9] tpm: fix driver so that burstcount can be safely ignored Alexander Steffen
2017-12-08 18:46 ` [RFC][PATCH 1/9] tpm_tis_core: clean up whitespace Alexander Steffen
2018-01-18 17:11   ` Jarkko Sakkinen
2017-12-08 18:46 ` [RFC][PATCH 2/9] tpm_tis_core: access single TIS registers before doing complex transfers Alexander Steffen
2018-01-18 17:16   ` Jarkko Sakkinen
2017-12-08 18:46 ` [RFC][PATCH 3/9] tpm_tis_core: correctly wait for flags to become zero Alexander Steffen
2018-01-18 17:49   ` Jarkko Sakkinen
2018-01-18 17:58   ` Jarkko Sakkinen
2018-01-18 17:59     ` Jarkko Sakkinen
2017-12-08 18:46 ` Alexander Steffen [this message]
2017-12-19  9:01   ` [RFC][PATCH 4/9] tpm_tis_core: send all data in single operation Nayna Jain
2018-01-18 18:04   ` Jarkko Sakkinen
2017-12-08 18:46 ` [RFC][PATCH 5/9] tpm_tis_core: use XDATA_FIFO for transfers if available Alexander Steffen
2018-01-18 18:20   ` Jarkko Sakkinen
2017-12-08 18:46 ` [RFC][PATCH 6/9] tpm_tis_spi: fix sending wrong data during wait state handling Alexander Steffen
2018-01-18 18:26   ` Jarkko Sakkinen
2017-12-08 18:46 ` [RFC][PATCH 7/9] tpm_tis_spi: release CS line when wait state handling fails Alexander Steffen
2018-01-18 18:30   ` Jarkko Sakkinen
2017-12-08 18:46 ` [RFC][PATCH 8/9] tpm_tis_spi: add delay between wait state retries Alexander Steffen
2018-01-11 19:46   ` Mimi Zohar
2018-01-12  8:28     ` Alexander Steffen
2018-01-12 14:53       ` Mimi Zohar
2018-01-15 22:30       ` Mimi Zohar
2018-01-17 17:15         ` Mimi Zohar
2018-01-17 18:58           ` Alexander Steffen
2018-01-18 18:32   ` Jarkko Sakkinen
2017-12-08 18:46 ` [RFC][PATCH 9/9] tpm: ignore burstcount to improve tpm_tis send() performance Alexander Steffen
2017-12-15 12:04 ` [RFC][PATCH 0/9] tpm: fix driver so that burstcount can be safely ignored Jarkko Sakkinen
2017-12-24 20:41   ` Jarkko Sakkinen
2018-01-04 13:11     ` Alexander.Steffen
2018-01-05  6:46       ` Nayna Jain
2018-01-05  7:38         ` Alexander Steffen
2018-01-08 10:50           ` Jarkko Sakkinen
2018-01-08 10:49       ` Jarkko Sakkinen
2017-12-19  8:53 ` Nayna Jain

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171208184658.9588-5-Alexander.Steffen@infineon.com \
    --to=alexander.steffen@infineon.com \
    --cc=jarkko.sakkinen@linux.intel.com \
    --cc=kgold@linux.vnet.ibm.com \
    --cc=linux-integrity@vger.kernel.org \
    --cc=nayna@linux.vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.