[PATCH v2 0/3] tpm2-cmd: Improve self test execution

[PATCH v2 0/3] tpm2-cmd: Improve self test execution

[Alexander Steffen]

The self test logic for TPM 2.0 was probably based on the implementation
for TPM 1.2, but did not correctly take into account some TPM 2.0 specifics.
This patch series fixes those issues.

v2:
- Moved implementation description from comment to commit message.

Alexander Steffen (3):
  tpm2-cmd: Trigger only missing self tests
  tpm2-cmd: Use dynamic delay to wait for self test result
  tpm2-cmd: React correctly to RC_TESTING from self tests

 drivers/char/tpm/tpm2-cmd.c | 69 +++++++++++++--------------------------------
 1 file changed, 20 insertions(+), 49 deletions(-)

-- 
2.7.4


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

[PATCH v2 1/3] tpm2-cmd: Trigger only missing self tests

[Alexander Steffen]

tpm2_do_selftest is only used during initialization of the TPM to ensure
that the device functions correctly. Therefore, it is sufficient to request
only missing self tests (parameter full_test=0), not a reexecution of all
self tests, as was done before. This allows for a faster execution of this
command.

Signed-off-by: Alexander Steffen <Alexander.Steffen-d0qZbvYSIPpWk0Htik3J/w@public.gmane.org>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 drivers/char/tpm/tpm2-cmd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index e1a41b7..8e940a5 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -865,7 +865,7 @@ static int tpm2_start_selftest(struct tpm_chip *chip, bool full)
 }
 
 /**
- * tpm2_do_selftest() - run a full self test
+ * tpm2_do_selftest() - ensure that all self tests have passed
  *
  * @chip: TPM chip to use
  *
@@ -886,7 +886,7 @@ static int tpm2_do_selftest(struct tpm_chip *chip)
 
 	loops = jiffies_to_msecs(duration) / delay_msec;
 
-	rc = tpm2_start_selftest(chip, true);
+	rc = tpm2_start_selftest(chip, false);
 	if (rc)
 		return rc;
 
-- 
2.7.4


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

[PATCH v2 2/3] tpm2-cmd: Use dynamic delay to wait for self test result

[Alexander Steffen]

In order to avoid delaying the code longer than necessary while still
giving the TPM enough time to execute the self tests asynchronously, start
with a small delay between two polls and increase it each round.

Signed-off-by: Alexander Steffen <Alexander.Steffen-d0qZbvYSIPpWk0Htik3J/w@public.gmane.org>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 drivers/char/tpm/tpm2-cmd.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 8e940a5..2178437 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -877,20 +877,17 @@ static int tpm2_start_selftest(struct tpm_chip *chip, bool full)
 static int tpm2_do_selftest(struct tpm_chip *chip)
 {
 	int rc;
-	unsigned int loops;
-	unsigned int delay_msec = 100;
-	unsigned long duration;
-	int i;
-
-	duration = tpm2_calc_ordinal_duration(chip, TPM2_CC_SELF_TEST);
+	unsigned int delay_msec = 20;
+	long duration;
 
-	loops = jiffies_to_msecs(duration) / delay_msec;
+	duration = jiffies_to_msecs(
+		tpm2_calc_ordinal_duration(chip, TPM2_CC_SELF_TEST));
 
 	rc = tpm2_start_selftest(chip, false);
 	if (rc)
 		return rc;
 
-	for (i = 0; i < loops; i++) {
+	while (duration > 0) {
 		/* Attempt to read a PCR value */
 		rc = tpm2_pcr_read(chip, 0, NULL);
 		if (rc < 0)
@@ -900,6 +897,10 @@ static int tpm2_do_selftest(struct tpm_chip *chip)
 			break;
 
 		tpm_msleep(delay_msec);
+		duration -= delay_msec;
+
+		/* wait longer the next round */
+		delay_msec *= 2;
 	}
 
 	return rc;
-- 
2.7.4


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

[PATCH v2 3/3] tpm2-cmd: React correctly to RC_TESTING from self tests

[Alexander Steffen]

The TPM can choose one of two ways to react to the TPM2_SelfTest command.
It can either run all self tests synchronously and then return RC_SUCCESS
once all tests were successful. Or it can choose to run the tests
asynchronously and return RC_TESTING immediately while the self tests still
execute in the background.

The previous implementation apparently was not aware of those possibilities
and attributed RC_TESTING to some prototype chips instead. With this change
the return code of TPM2_SelfTest is interpreted correctly, i.e. the self
test result is polled if and only if RC_TESTING is received.

Unfortunately, the polling cannot be done in the most straightforward way.
If RC_TESTING is received, ideally the code should now poll the
selfTestDone bit in the STS register, as this avoids sending more commands,
that might interrupt self tests executing in the background and thus
prevent them from ever completing. But it cannot be guaranteed that this
bit is correctly implemented for all devices, so the next best thing would
be to use TPM2_GetTestResult to query the test result. But the response to
that command can be very long, and the code currently lacks the
capabilities for efficient unmarshalling, so it is difficult to execute
this command.

Therefore, we simply run the TPM2_SelfTest command in a loop, which should
complete eventually, since we only request the execution of self tests that
have not yet been done.

Signed-off-by: Alexander Steffen <Alexander.Steffen-d0qZbvYSIPpWk0Htik3J/w@public.gmane.org>
---
 drivers/char/tpm/tpm2-cmd.c | 52 ++++++++++-----------------------------------
 1 file changed, 11 insertions(+), 41 deletions(-)

diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 2178437..70ee328 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -834,64 +834,34 @@ static const struct tpm_input_header tpm2_selftest_header = {
 };
 
 /**
- * tpm2_continue_selftest() - start a self test
- *
- * @chip: TPM chip to use
- * @full: test all commands instead of testing only those that were not
- *        previously tested.
- *
- * Return: Same as with tpm_transmit_cmd with exception of RC_TESTING.
- */
-static int tpm2_start_selftest(struct tpm_chip *chip, bool full)
-{
-	int rc;
-	struct tpm2_cmd cmd;
-
-	cmd.header.in = tpm2_selftest_header;
-	cmd.params.selftest_in.full_test = full;
-
-	rc = tpm_transmit_cmd(chip, NULL, &cmd, TPM2_SELF_TEST_IN_SIZE, 0, 0,
-			      "continue selftest");
-
-	/* At least some prototype chips seem to give RC_TESTING error
-	 * immediately. This is a workaround for that.
-	 */
-	if (rc == TPM2_RC_TESTING) {
-		dev_warn(&chip->dev, "Got RC_TESTING, ignoring\n");
-		rc = 0;
-	}
-
-	return rc;
-}
-
-/**
  * tpm2_do_selftest() - ensure that all self tests have passed
  *
  * @chip: TPM chip to use
  *
  * Return: Same as with tpm_transmit_cmd.
  *
- * During the self test TPM2 commands return with the error code RC_TESTING.
- * Waiting is done by issuing PCR read until it executes successfully.
+ * The TPM can either run all self tests synchronously and then return
+ * RC_SUCCESS once all tests were successful. Or it can choose to run the tests
+ * asynchronously and return RC_TESTING immediately while the self tests still
+ * execute in the background. This function handles both cases and waits until
+ * all tests have completed.
  */
 static int tpm2_do_selftest(struct tpm_chip *chip)
 {
 	int rc;
 	unsigned int delay_msec = 20;
 	long duration;
+	struct tpm2_cmd cmd;
 
 	duration = jiffies_to_msecs(
 		tpm2_calc_ordinal_duration(chip, TPM2_CC_SELF_TEST));
 
-	rc = tpm2_start_selftest(chip, false);
-	if (rc)
-		return rc;
-
 	while (duration > 0) {
-		/* Attempt to read a PCR value */
-		rc = tpm2_pcr_read(chip, 0, NULL);
-		if (rc < 0)
-			break;
+		cmd.header.in = tpm2_selftest_header;
+		cmd.params.selftest_in.full_test = 0;
+
+		rc = tpm_transmit_cmd(chip, NULL, &cmd, TPM2_SELF_TEST_IN_SIZE,
+				      0, 0, "continue selftest");
 
 		if (rc != TPM2_RC_TESTING)
 			break;
-- 
2.7.4


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH v2 3/3] tpm2-cmd: React correctly to RC_TESTING from self tests

[Jarkko Sakkinen]

On Thu, Aug 24, 2017 at 10:34:10AM +0200, Alexander Steffen wrote:
> The TPM can choose one of two ways to react to the TPM2_SelfTest command.
> It can either run all self tests synchronously and then return RC_SUCCESS
> once all tests were successful. Or it can choose to run the tests
> asynchronously and return RC_TESTING immediately while the self tests still
> execute in the background.
> 
> The previous implementation apparently was not aware of those possibilities
> and attributed RC_TESTING to some prototype chips instead. With this change
> the return code of TPM2_SelfTest is interpreted correctly, i.e. the self
> test result is polled if and only if RC_TESTING is received.
> 
> Unfortunately, the polling cannot be done in the most straightforward way.
> If RC_TESTING is received, ideally the code should now poll the
> selfTestDone bit in the STS register, as this avoids sending more commands,
> that might interrupt self tests executing in the background and thus
> prevent them from ever completing. But it cannot be guaranteed that this
> bit is correctly implemented for all devices, so the next best thing would
> be to use TPM2_GetTestResult to query the test result. But the response to
> that command can be very long, and the code currently lacks the
> capabilities for efficient unmarshalling, so it is difficult to execute
> this command.
> 
> Therefore, we simply run the TPM2_SelfTest command in a loop, which should
> complete eventually, since we only request the execution of self tests that
> have not yet been done.
> 
> Signed-off-by: Alexander Steffen <Alexander.Steffen-d0qZbvYSIPpWk0Htik3J/w@public.gmane.org>
> ---
>  drivers/char/tpm/tpm2-cmd.c | 52 ++++++++++-----------------------------------
>  1 file changed, 11 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> index 2178437..70ee328 100644
> --- a/drivers/char/tpm/tpm2-cmd.c
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -834,64 +834,34 @@ static const struct tpm_input_header tpm2_selftest_header = {
>  };
>  
>  /**
> - * tpm2_continue_selftest() - start a self test
> - *
> - * @chip: TPM chip to use
> - * @full: test all commands instead of testing only those that were not
> - *        previously tested.
> - *
> - * Return: Same as with tpm_transmit_cmd with exception of RC_TESTING.
> - */
> -static int tpm2_start_selftest(struct tpm_chip *chip, bool full)
> -{
> -	int rc;
> -	struct tpm2_cmd cmd;
> -
> -	cmd.header.in = tpm2_selftest_header;
> -	cmd.params.selftest_in.full_test = full;
> -
> -	rc = tpm_transmit_cmd(chip, NULL, &cmd, TPM2_SELF_TEST_IN_SIZE, 0, 0,
> -			      "continue selftest");
> -
> -	/* At least some prototype chips seem to give RC_TESTING error
> -	 * immediately. This is a workaround for that.
> -	 */
> -	if (rc == TPM2_RC_TESTING) {
> -		dev_warn(&chip->dev, "Got RC_TESTING, ignoring\n");
> -		rc = 0;
> -	}
> -
> -	return rc;
> -}
> -
> -/**
>   * tpm2_do_selftest() - ensure that all self tests have passed
>   *
>   * @chip: TPM chip to use
>   *
>   * Return: Same as with tpm_transmit_cmd.
>   *
> - * During the self test TPM2 commands return with the error code RC_TESTING.
> - * Waiting is done by issuing PCR read until it executes successfully.
> + * The TPM can either run all self tests synchronously and then return
> + * RC_SUCCESS once all tests were successful. Or it can choose to run the tests
> + * asynchronously and return RC_TESTING immediately while the self tests still
> + * execute in the background. This function handles both cases and waits until
> + * all tests have completed.
>   */
>  static int tpm2_do_selftest(struct tpm_chip *chip)
>  {
>  	int rc;
>  	unsigned int delay_msec = 20;
>  	long duration;
> +	struct tpm2_cmd cmd;
>  
>  	duration = jiffies_to_msecs(
>  		tpm2_calc_ordinal_duration(chip, TPM2_CC_SELF_TEST));
>  
> -	rc = tpm2_start_selftest(chip, false);
> -	if (rc)
> -		return rc;
> -
>  	while (duration > 0) {
> -		/* Attempt to read a PCR value */
> -		rc = tpm2_pcr_read(chip, 0, NULL);
> -		if (rc < 0)
> -			break;
> +		cmd.header.in = tpm2_selftest_header;
> +		cmd.params.selftest_in.full_test = 0;
> +
> +		rc = tpm_transmit_cmd(chip, NULL, &cmd, TPM2_SELF_TEST_IN_SIZE,
> +				      0, 0, "continue selftest");
>  
>  		if (rc != TPM2_RC_TESTING)
>  			break;
> -- 
> 2.7.4
> 

Much better! Thank you.

Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

PS. I'm not subscribed at the moment to tpmdd mailing list because SF
kicked me out of the list. I've been trying to contact vger.kernel.org
to create a new ML there but haven't received any response. I just saw
your resends so it might have something to do with this. Also, patchwork
is broken ATM:

https://patchwork.kernel.org/project/tpmdd-devel/list/

/Jarkko

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH v2 0/3] tpm2-cmd: Improve self test execution

[Jarkko Sakkinen]

On Thu, Aug 24, 2017 at 10:30:03AM +0200, Alexander Steffen wrote:
> The self test logic for TPM 2.0 was probably based on the implementation
> for TPM 1.2, but did not correctly take into account some TPM 2.0 specifics.
> This patch series fixes those issues.
> 
> v2:
> - Moved implementation description from comment to commit message.
> 
> Alexander Steffen (3):
>   tpm2-cmd: Trigger only missing self tests
>   tpm2-cmd: Use dynamic delay to wait for self test result
>   tpm2-cmd: React correctly to RC_TESTING from self tests
> 
>  drivers/char/tpm/tpm2-cmd.c | 69 +++++++++++++--------------------------------
>  1 file changed, 20 insertions(+), 49 deletions(-)
> 
> -- 
> 2.7.4
> 

Missing stuff from CC.

/Jarkko

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot