[PATCH 1/2] tpm: Separate out the TPM tests for v1 and v2

[PATCH 1/2] tpm: Separate out the TPM tests for v1 and v2

[Simon Glass]

Currently there is only one test and it only works on TPM v2. Update it
to work on v1.2 as well, using a new function to pick up the required
TPM.

Update sandbox to include both a v1.2 and v2 TPM so that this works.
Split out the existing test into two pieces, one for init and one for
the v2-only report_state feature.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 arch/sandbox/dts/test.dts |  4 +++
 test/dm/tpm.c             | 60 +++++++++++++++++++++++++++++++++------
 2 files changed, 55 insertions(+), 9 deletions(-)

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index 05e09128a38..d72d7a567a7 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -1367,6 +1367,10 @@
 		compatible = "sandbox,tpm2";
 	};
 
+	tpm {
+		compatible = "google,sandbox-tpm";
+	};
+
 	uart0: serial {
 		compatible = "sandbox,serial";
 		bootph-all;
diff --git a/test/dm/tpm.c b/test/dm/tpm.c
index dca540bb561..7d880012090 100644
--- a/test/dm/tpm.c
+++ b/test/dm/tpm.c
@@ -11,24 +11,66 @@
 #include <test/test.h>
 #include <test/ut.h>
 
-/* Basic test of the TPM uclass */
+/*
+ * get_tpm_version() - Get a TPM of the given version
+ *
+ * @version: Version to get
+ * @devp: Returns the TPM device
+ * Returns: 0 if OK, -ENODEV if not found
+ */
+static int get_tpm_version(enum tpm_version version, struct udevice **devp)
+{
+	struct udevice *dev;
+
+	/*
+	 * For now we have to probe each TPM, since the version is set up in
+	 * of_to_plat(). We could require TPMs to declare their version when
+	 * probed, to avoid this
+	 */
+	uclass_foreach_dev_probe(UCLASS_TPM, dev) {
+		if (tpm_get_version(dev) == version) {
+			*devp = dev;
+			return 0;
+		}
+	}
+
+	return -ENODEV;
+}
+
+/* Basic test of initing a TPM */
+static int test_tpm_init(struct unit_test_state *uts, enum tpm_version version)
+{
+	struct udevice *dev;
+
+	/* check probe success */
+	ut_assertok(get_tpm_version(version, &dev));
+
+	ut_assertok(tpm_init(dev));
+
+	return 0;
+}
+
 static int dm_test_tpm(struct unit_test_state *uts)
+{
+	ut_assertok(test_tpm_init(uts, TPM_V1));
+	ut_assertok(test_tpm_init(uts, TPM_V2));
+
+	return 0;
+}
+DM_TEST(dm_test_tpm, UT_TESTF_SCAN_FDT);
+
+/* Test report_state */
+static int dm_test_tpm_report_state(struct unit_test_state *uts)
 {
 	struct udevice *dev;
 	char buf[50];
 
 	/* check probe success */
-	ut_assertok(uclass_first_device_err(UCLASS_TPM, &dev));
-	ut_assert(tpm_is_v2(dev));
+	ut_assertok(get_tpm_version(TPM_V2, &dev));
 
 	ut_assert(tpm_report_state(dev, buf, sizeof(buf)));
 	ut_asserteq_str("init_done=0", buf);
 
-	ut_assertok(tpm_init(dev));
-	 /*
-	  * tpm_auto_start will rerun tpm_init, but handles the
-	  * -EBUSY return code internally.
-	  */
 	ut_assertok(tpm_auto_start(dev));
 
 	ut_assert(tpm_report_state(dev, buf, sizeof(buf)));
@@ -36,4 +78,4 @@ static int dm_test_tpm(struct unit_test_state *uts)
 
 	return 0;
 }
-DM_TEST(dm_test_tpm, UT_TESTF_SCAN_FDT);
+DM_TEST(dm_test_tpm_report_state, UT_TESTF_SCAN_FDT);
-- 
2.39.2.637.g21b0678d19-goog

[PATCH 2/2] tpm: Implement tpm_auto_start() for TPMv1.2

[Simon Glass]

Add an implementation of this, moving the common call to tpm_init() up
into the common API implementation.

Add a test.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 include/tpm-common.h |  2 +-
 include/tpm-v1.h     | 11 +++++++++++
 lib/tpm-v1.c         | 16 ++++++++++++++++
 lib/tpm-v2.c         |  8 --------
 lib/tpm_api.c        | 19 ++++++++++++++++---
 test/dm/tpm.c        | 45 ++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 89 insertions(+), 12 deletions(-)

diff --git a/include/tpm-common.h b/include/tpm-common.h
index b2c5404430f..1ba81386ce1 100644
--- a/include/tpm-common.h
+++ b/include/tpm-common.h
@@ -94,7 +94,7 @@ struct tpm_ops {
 	 * close().
 	 *
 	 * @dev:	Device to open
-	 * @return 0 ok OK, -ve on error
+	 * @return 0 ok OK, -EBUSY if already opened, other -ve on other error
 	 */
 	int (*open)(struct udevice *dev);
 
diff --git a/include/tpm-v1.h b/include/tpm-v1.h
index 33d53fb695e..60b71e2a4b6 100644
--- a/include/tpm-v1.h
+++ b/include/tpm-v1.h
@@ -591,4 +591,15 @@ u32 tpm_set_global_lock(struct udevice *dev);
  */
 u32 tpm1_resume(struct udevice *dev);
 
+/**
+ * tpm1_auto_start() - start up the TPM
+ *
+ * This does not do a self test.
+ *
+ * @dev		TPM device
+ * Return: TPM2_RC_SUCCESS, on success, or when the TPM returns
+ * TPM_INVALID_POSTINIT; TPM_FAILEDSELFTEST, if the TPM is in failure state
+ */
+u32 tpm1_auto_start(struct udevice *dev);
+
 #endif /* __TPM_V1_H */
diff --git a/lib/tpm-v1.c b/lib/tpm-v1.c
index d0e3ab1b21d..ea3833549bc 100644
--- a/lib/tpm-v1.c
+++ b/lib/tpm-v1.c
@@ -69,6 +69,22 @@ u32 tpm1_continue_self_test(struct udevice *dev)
 	return tpm_sendrecv_command(dev, command, NULL, NULL);
 }
 
+u32 tpm1_auto_start(struct udevice *dev)
+{
+	u32 rc;
+
+	rc = tpm1_startup(dev, TPM_ST_CLEAR);
+	/* continue on if the TPM is already inited */
+	if (rc && rc != TPM_INVALID_POSTINIT)
+		return rc;
+
+	rc = tpm1_self_test_full(dev);
+	if (rc)
+		return rc;
+
+	return rc;
+}
+
 u32 tpm1_clear_and_reenable(struct udevice *dev)
 {
 	u32 ret;
diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c
index 895b093bcb1..9ab5b46df17 100644
--- a/lib/tpm-v2.c
+++ b/lib/tpm-v2.c
@@ -48,14 +48,6 @@ u32 tpm2_auto_start(struct udevice *dev)
 {
 	u32 rc;
 
-	/*
-	 * the tpm_init() will return -EBUSY if the init has already happened
-	 * The selftest and startup code can run multiple times with no side
-	 * effects
-	 */
-	rc = tpm_init(dev);
-	if (rc && rc != -EBUSY)
-		return rc;
 	rc = tpm2_self_test(dev, TPMI_YES);
 
 	if (rc == TPM2_RC_INITIALIZE) {
diff --git a/lib/tpm_api.c b/lib/tpm_api.c
index 5b2c11a277c..3ef5e811794 100644
--- a/lib/tpm_api.c
+++ b/lib/tpm_api.c
@@ -37,10 +37,23 @@ u32 tpm_startup(struct udevice *dev, enum tpm_startup_type mode)
 
 u32 tpm_auto_start(struct udevice *dev)
 {
-	if (tpm_is_v2(dev))
-		return tpm2_auto_start(dev);
+	u32 rc;
 
-	return -ENOSYS;
+	/*
+	 * the tpm_init() will return -EBUSY if the init has already happened
+	 * The selftest and startup code can run multiple times with no side
+	 * effects
+	 */
+	rc = tpm_init(dev);
+	if (rc && rc != -EBUSY)
+		return rc;
+
+	if (tpm_is_v1(dev))
+		return tpm1_auto_start(dev);
+	else if (tpm_is_v2(dev))
+		return tpm2_auto_start(dev);
+	else
+		return -ENOSYS;
 }
 
 u32 tpm_resume(struct udevice *dev)
diff --git a/test/dm/tpm.c b/test/dm/tpm.c
index 7d880012090..3defb3c3da1 100644
--- a/test/dm/tpm.c
+++ b/test/dm/tpm.c
@@ -79,3 +79,48 @@ static int dm_test_tpm_report_state(struct unit_test_state *uts)
 	return 0;
 }
 DM_TEST(dm_test_tpm_report_state, UT_TESTF_SCAN_FDT);
+
+/**
+ * test_tpm_autostart() - check the tpm_auto_start() call
+ *
+ * @uts: Unit test state
+ * @version: TPM version to use
+ * @reinit: true to call tpm_init() first
+ * Returns 0 if OK, non-zero on failure
+ */
+static int test_tpm_autostart(struct unit_test_state *uts,
+			      enum tpm_version version, bool reinit)
+{
+	struct udevice *dev;
+
+	/* check probe success */
+	ut_assertok(get_tpm_version(version, &dev));
+
+	if (reinit)
+		ut_assertok(tpm_init(dev));
+	 /*
+	  * tpm_auto_start will rerun tpm_init() if reinit, but handles the
+	  * -EBUSY return code internally.
+	  */
+	ut_assertok(tpm_auto_start(dev));
+
+	return 0;
+}
+
+static int dm_test_tpm_autostart(struct unit_test_state *uts)
+{
+	ut_assertok(test_tpm_autostart(uts, TPM_V1, false));
+	ut_assertok(test_tpm_autostart(uts, TPM_V2, false));
+
+	return 0;
+}
+DM_TEST(dm_test_tpm_autostart, UT_TESTF_SCAN_FDT);
+
+static int dm_test_tpm_autostart_reinit(struct unit_test_state *uts)
+{
+	ut_assertok(test_tpm_autostart(uts, TPM_V1, true));
+	ut_assertok(test_tpm_autostart(uts, TPM_V2, true));
+
+	return 0;
+}
+DM_TEST(dm_test_tpm_autostart_reinit, UT_TESTF_SCAN_FDT);
-- 
2.39.2.637.g21b0678d19-goog

Re: [PATCH 2/2] tpm: Implement tpm_auto_start() for TPMv1.2

[Ilias Apalodimas]

Hi Simon,

On Mon, Feb 20, 2023 at 02:27:36PM -0700, Simon Glass wrote:
> Add an implementation of this, moving the common call to tpm_init() up
> into the common API implementation.
>
> Add a test.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>  include/tpm-common.h |  2 +-
>  include/tpm-v1.h     | 11 +++++++++++
>  lib/tpm-v1.c         | 16 ++++++++++++++++
>  lib/tpm-v2.c         |  8 --------
>  lib/tpm_api.c        | 19 ++++++++++++++++---
>  test/dm/tpm.c        | 45 ++++++++++++++++++++++++++++++++++++++++++++
>  6 files changed, 89 insertions(+), 12 deletions(-)
>
> diff --git a/include/tpm-common.h b/include/tpm-common.h
> index b2c5404430f..1ba81386ce1 100644
> --- a/include/tpm-common.h
> +++ b/include/tpm-common.h
> @@ -94,7 +94,7 @@ struct tpm_ops {
>  	 * close().
>  	 *
>  	 * @dev:	Device to open
> -	 * @return 0 ok OK, -ve on error
> +	 * @return 0 ok OK, -EBUSY if already opened, other -ve on other error
>  	 */
>  	int (*open)(struct udevice *dev);
>
> diff --git a/include/tpm-v1.h b/include/tpm-v1.h
> index 33d53fb695e..60b71e2a4b6 100644
> --- a/include/tpm-v1.h
> +++ b/include/tpm-v1.h
> @@ -591,4 +591,15 @@ u32 tpm_set_global_lock(struct udevice *dev);
>   */
>  u32 tpm1_resume(struct udevice *dev);
>
> +/**
> + * tpm1_auto_start() - start up the TPM
> + *
> + * This does not do a self test.
> + *
> + * @dev		TPM device
> + * Return: TPM2_RC_SUCCESS, on success, or when the TPM returns
> + * TPM_INVALID_POSTINIT; TPM_FAILEDSELFTEST, if the TPM is in failure state
> + */
> +u32 tpm1_auto_start(struct udevice *dev);
> +
>  #endif /* __TPM_V1_H */
> diff --git a/lib/tpm-v1.c b/lib/tpm-v1.c
> index d0e3ab1b21d..ea3833549bc 100644
> --- a/lib/tpm-v1.c
> +++ b/lib/tpm-v1.c
> @@ -69,6 +69,22 @@ u32 tpm1_continue_self_test(struct udevice *dev)
>  	return tpm_sendrecv_command(dev, command, NULL, NULL);
>  }
>
> +u32 tpm1_auto_start(struct udevice *dev)
> +{
> +	u32 rc;
> +
> +	rc = tpm1_startup(dev, TPM_ST_CLEAR);
> +	/* continue on if the TPM is already inited */
> +	if (rc && rc != TPM_INVALID_POSTINIT)
> +		return rc;
> +
> +	rc = tpm1_self_test_full(dev);
> +	if (rc)
> +		return rc;

Mind if I remove this if before merging?

> +
> +	return rc;
> +}
> +
>  {

[...]

> +
> +	return 0;
> +}
> +DM_TEST(dm_test_tpm_autostart_reinit, UT_TESTF_SCAN_FDT);
> --
> 2.39.2.637.g21b0678d19-goog
>

Other than that
Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>

Re: [PATCH 2/2] tpm: Implement tpm_auto_start() for TPMv1.2

[Simon Glass]

Hi Ilias,

On Tue, 21 Feb 2023 at 06:03, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> Hi Simon,
>
> On Mon, Feb 20, 2023 at 02:27:36PM -0700, Simon Glass wrote:
> > Add an implementation of this, moving the common call to tpm_init() up
> > into the common API implementation.
> >
> > Add a test.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> >  include/tpm-common.h |  2 +-
> >  include/tpm-v1.h     | 11 +++++++++++
> >  lib/tpm-v1.c         | 16 ++++++++++++++++
> >  lib/tpm-v2.c         |  8 --------
> >  lib/tpm_api.c        | 19 ++++++++++++++++---
> >  test/dm/tpm.c        | 45 ++++++++++++++++++++++++++++++++++++++++++++
> >  6 files changed, 89 insertions(+), 12 deletions(-)
> >
> > diff --git a/include/tpm-common.h b/include/tpm-common.h
> > index b2c5404430f..1ba81386ce1 100644
> > --- a/include/tpm-common.h
> > +++ b/include/tpm-common.h
> > @@ -94,7 +94,7 @@ struct tpm_ops {
> >        * close().
> >        *
> >        * @dev:        Device to open
> > -      * @return 0 ok OK, -ve on error
> > +      * @return 0 ok OK, -EBUSY if already opened, other -ve on other error
> >        */
> >       int (*open)(struct udevice *dev);
> >
> > diff --git a/include/tpm-v1.h b/include/tpm-v1.h
> > index 33d53fb695e..60b71e2a4b6 100644
> > --- a/include/tpm-v1.h
> > +++ b/include/tpm-v1.h
> > @@ -591,4 +591,15 @@ u32 tpm_set_global_lock(struct udevice *dev);
> >   */
> >  u32 tpm1_resume(struct udevice *dev);
> >
> > +/**
> > + * tpm1_auto_start() - start up the TPM
> > + *
> > + * This does not do a self test.
> > + *
> > + * @dev              TPM device
> > + * Return: TPM2_RC_SUCCESS, on success, or when the TPM returns
> > + * TPM_INVALID_POSTINIT; TPM_FAILEDSELFTEST, if the TPM is in failure state
> > + */
> > +u32 tpm1_auto_start(struct udevice *dev);
> > +
> >  #endif /* __TPM_V1_H */
> > diff --git a/lib/tpm-v1.c b/lib/tpm-v1.c
> > index d0e3ab1b21d..ea3833549bc 100644
> > --- a/lib/tpm-v1.c
> > +++ b/lib/tpm-v1.c
> > @@ -69,6 +69,22 @@ u32 tpm1_continue_self_test(struct udevice *dev)
> >       return tpm_sendrecv_command(dev, command, NULL, NULL);
> >  }
> >
> > +u32 tpm1_auto_start(struct udevice *dev)
> > +{
> > +     u32 rc;
> > +
> > +     rc = tpm1_startup(dev, TPM_ST_CLEAR);
> > +     /* continue on if the TPM is already inited */
> > +     if (rc && rc != TPM_INVALID_POSTINIT)
> > +             return rc;
> > +
> > +     rc = tpm1_self_test_full(dev);
> > +     if (rc)
> > +             return rc;
>
> Mind if I remove this if before merging?

Yes that's fine.

>
> > +
> > +     return rc;
> > +}
> > +
> >  {
>
> [...]
>
> > +
> > +     return 0;
> > +}
> > +DM_TEST(dm_test_tpm_autostart_reinit, UT_TESTF_SCAN_FDT);
> > --
> > 2.39.2.637.g21b0678d19-goog
> >
>
> Other than that
> Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
>

Regards,
Simon

Re: [PATCH 2/2] tpm: Implement tpm_auto_start() for TPMv1.2

[Ilias Apalodimas]

Hi Simon,

Unfortunately, this doesn't apply cleanly over
https://lore.kernel.org/u-boot/20230218152741.528191-1-ilias.apalodimas@linaro.org/

I'll have a look at the conflicts, if they are minor i'll fix them up.
Otherwise you'll have to respin this

Thanks
/Ilias

On Tue, 21 Feb 2023 at 15:08, Simon Glass <sjg@chromium.org> wrote:
>
> Hi Ilias,
>
> On Tue, 21 Feb 2023 at 06:03, Ilias Apalodimas
> <ilias.apalodimas@linaro.org> wrote:
> >
> > Hi Simon,
> >
> > On Mon, Feb 20, 2023 at 02:27:36PM -0700, Simon Glass wrote:
> > > Add an implementation of this, moving the common call to tpm_init() up
> > > into the common API implementation.
> > >
> > > Add a test.
> > >
> > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > ---
> > >
> > >  include/tpm-common.h |  2 +-
> > >  include/tpm-v1.h     | 11 +++++++++++
> > >  lib/tpm-v1.c         | 16 ++++++++++++++++
> > >  lib/tpm-v2.c         |  8 --------
> > >  lib/tpm_api.c        | 19 ++++++++++++++++---
> > >  test/dm/tpm.c        | 45 ++++++++++++++++++++++++++++++++++++++++++++
> > >  6 files changed, 89 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/include/tpm-common.h b/include/tpm-common.h
> > > index b2c5404430f..1ba81386ce1 100644
> > > --- a/include/tpm-common.h
> > > +++ b/include/tpm-common.h
> > > @@ -94,7 +94,7 @@ struct tpm_ops {
> > >        * close().
> > >        *
> > >        * @dev:        Device to open
> > > -      * @return 0 ok OK, -ve on error
> > > +      * @return 0 ok OK, -EBUSY if already opened, other -ve on other error
> > >        */
> > >       int (*open)(struct udevice *dev);
> > >
> > > diff --git a/include/tpm-v1.h b/include/tpm-v1.h
> > > index 33d53fb695e..60b71e2a4b6 100644
> > > --- a/include/tpm-v1.h
> > > +++ b/include/tpm-v1.h
> > > @@ -591,4 +591,15 @@ u32 tpm_set_global_lock(struct udevice *dev);
> > >   */
> > >  u32 tpm1_resume(struct udevice *dev);
> > >
> > > +/**
> > > + * tpm1_auto_start() - start up the TPM
> > > + *
> > > + * This does not do a self test.
> > > + *
> > > + * @dev              TPM device
> > > + * Return: TPM2_RC_SUCCESS, on success, or when the TPM returns
> > > + * TPM_INVALID_POSTINIT; TPM_FAILEDSELFTEST, if the TPM is in failure state
> > > + */
> > > +u32 tpm1_auto_start(struct udevice *dev);
> > > +
> > >  #endif /* __TPM_V1_H */
> > > diff --git a/lib/tpm-v1.c b/lib/tpm-v1.c
> > > index d0e3ab1b21d..ea3833549bc 100644
> > > --- a/lib/tpm-v1.c
> > > +++ b/lib/tpm-v1.c
> > > @@ -69,6 +69,22 @@ u32 tpm1_continue_self_test(struct udevice *dev)
> > >       return tpm_sendrecv_command(dev, command, NULL, NULL);
> > >  }
> > >
> > > +u32 tpm1_auto_start(struct udevice *dev)
> > > +{
> > > +     u32 rc;
> > > +
> > > +     rc = tpm1_startup(dev, TPM_ST_CLEAR);
> > > +     /* continue on if the TPM is already inited */
> > > +     if (rc && rc != TPM_INVALID_POSTINIT)
> > > +             return rc;
> > > +
> > > +     rc = tpm1_self_test_full(dev);
> > > +     if (rc)
> > > +             return rc;
> >
> > Mind if I remove this if before merging?
>
> Yes that's fine.
>
> >
> > > +
> > > +     return rc;
> > > +}
> > > +
> > >  {
> >
> > [...]
> >
> > > +
> > > +     return 0;
> > > +}
> > > +DM_TEST(dm_test_tpm_autostart_reinit, UT_TESTF_SCAN_FDT);
> > > --
> > > 2.39.2.637.g21b0678d19-goog
> > >
> >
> > Other than that
> > Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> >
>
> Regards,
> Simon

Re: [PATCH 2/2] tpm: Implement tpm_auto_start() for TPMv1.2

[Simon Glass]

Hi Ilias,

On Tue, 21 Feb 2023 at 06:10, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> Hi Simon,
>
> Unfortunately, this doesn't apply cleanly over
> https://lore.kernel.org/u-boot/20230218152741.528191-1-ilias.apalodimas@linaro.org/
>
> I'll have a look at the conflicts, if they are minor i'll fix them up.
> Otherwise you'll have to respin this

That's strange. I just tried again with going to the patch [1] and
downloading the series. I am using -next though...could that be why?

Regards,
Simon

[1] https://patchwork.ozlabs.org/project/uboot/patch/20230218152741.528191-4-ilias.apalodimas@linaro.org/

>
> Thanks
> /Ilias
>
> On Tue, 21 Feb 2023 at 15:08, Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Ilias,
> >
> > On Tue, 21 Feb 2023 at 06:03, Ilias Apalodimas
> > <ilias.apalodimas@linaro.org> wrote:
> > >
> > > Hi Simon,
> > >
> > > On Mon, Feb 20, 2023 at 02:27:36PM -0700, Simon Glass wrote:
> > > > Add an implementation of this, moving the common call to tpm_init() up
> > > > into the common API implementation.
> > > >
> > > > Add a test.
> > > >
> > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > ---
> > > >
> > > >  include/tpm-common.h |  2 +-
> > > >  include/tpm-v1.h     | 11 +++++++++++
> > > >  lib/tpm-v1.c         | 16 ++++++++++++++++
> > > >  lib/tpm-v2.c         |  8 --------
> > > >  lib/tpm_api.c        | 19 ++++++++++++++++---
> > > >  test/dm/tpm.c        | 45 ++++++++++++++++++++++++++++++++++++++++++++
> > > >  6 files changed, 89 insertions(+), 12 deletions(-)
> > > >
> > > > diff --git a/include/tpm-common.h b/include/tpm-common.h
> > > > index b2c5404430f..1ba81386ce1 100644
> > > > --- a/include/tpm-common.h
> > > > +++ b/include/tpm-common.h
> > > > @@ -94,7 +94,7 @@ struct tpm_ops {
> > > >        * close().
> > > >        *
> > > >        * @dev:        Device to open
> > > > -      * @return 0 ok OK, -ve on error
> > > > +      * @return 0 ok OK, -EBUSY if already opened, other -ve on other error
> > > >        */
> > > >       int (*open)(struct udevice *dev);
> > > >
> > > > diff --git a/include/tpm-v1.h b/include/tpm-v1.h
> > > > index 33d53fb695e..60b71e2a4b6 100644
> > > > --- a/include/tpm-v1.h
> > > > +++ b/include/tpm-v1.h
> > > > @@ -591,4 +591,15 @@ u32 tpm_set_global_lock(struct udevice *dev);
> > > >   */
> > > >  u32 tpm1_resume(struct udevice *dev);
> > > >
> > > > +/**
> > > > + * tpm1_auto_start() - start up the TPM
> > > > + *
> > > > + * This does not do a self test.
> > > > + *
> > > > + * @dev              TPM device
> > > > + * Return: TPM2_RC_SUCCESS, on success, or when the TPM returns
> > > > + * TPM_INVALID_POSTINIT; TPM_FAILEDSELFTEST, if the TPM is in failure state
> > > > + */
> > > > +u32 tpm1_auto_start(struct udevice *dev);
> > > > +
> > > >  #endif /* __TPM_V1_H */
> > > > diff --git a/lib/tpm-v1.c b/lib/tpm-v1.c
> > > > index d0e3ab1b21d..ea3833549bc 100644
> > > > --- a/lib/tpm-v1.c
> > > > +++ b/lib/tpm-v1.c
> > > > @@ -69,6 +69,22 @@ u32 tpm1_continue_self_test(struct udevice *dev)
> > > >       return tpm_sendrecv_command(dev, command, NULL, NULL);
> > > >  }
> > > >
> > > > +u32 tpm1_auto_start(struct udevice *dev)
> > > > +{
> > > > +     u32 rc;
> > > > +
> > > > +     rc = tpm1_startup(dev, TPM_ST_CLEAR);
> > > > +     /* continue on if the TPM is already inited */
> > > > +     if (rc && rc != TPM_INVALID_POSTINIT)
> > > > +             return rc;
> > > > +
> > > > +     rc = tpm1_self_test_full(dev);
> > > > +     if (rc)
> > > > +             return rc;
> > >
> > > Mind if I remove this if before merging?
> >
> > Yes that's fine.
> >
> > >
> > > > +
> > > > +     return rc;
> > > > +}
> > > > +
> > > >  {
> > >
> > > [...]
> > >
> > > > +
> > > > +     return 0;
> > > > +}
> > > > +DM_TEST(dm_test_tpm_autostart_reinit, UT_TESTF_SCAN_FDT);
> > > > --
> > > > 2.39.2.637.g21b0678d19-goog
> > > >
> > >
> > > Other than that
> > > Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > >
> >
> > Regards,
> > Simon

Re: [PATCH 1/2] tpm: Separate out the TPM tests for v1 and v2

[Ilias Apalodimas]

On Mon, Feb 20, 2023 at 02:27:35PM -0700, Simon Glass wrote:
> Currently there is only one test and it only works on TPM v2. Update it
> to work on v1.2 as well, using a new function to pick up the required
> TPM.
>
> Update sandbox to include both a v1.2 and v2 TPM so that this works.
> Split out the existing test into two pieces, one for init and one for
> the v2-only report_state feature.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>  arch/sandbox/dts/test.dts |  4 +++
>  test/dm/tpm.c             | 60 +++++++++++++++++++++++++++++++++------
>  2 files changed, 55 insertions(+), 9 deletions(-)
>
> diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
> index 05e09128a38..d72d7a567a7 100644
> --- a/arch/sandbox/dts/test.dts
> +++ b/arch/sandbox/dts/test.dts
> @@ -1367,6 +1367,10 @@
>  		compatible = "sandbox,tpm2";
>  	};
>
> +	tpm {
> +		compatible = "google,sandbox-tpm";
> +	};
> +
>  	uart0: serial {
>  		compatible = "sandbox,serial";
>  		bootph-all;
> diff --git a/test/dm/tpm.c b/test/dm/tpm.c
> index dca540bb561..7d880012090 100644
> --- a/test/dm/tpm.c
> +++ b/test/dm/tpm.c
> @@ -11,24 +11,66 @@
>  #include <test/test.h>
>  #include <test/ut.h>
>
> -/* Basic test of the TPM uclass */
> +/*
> + * get_tpm_version() - Get a TPM of the given version
> + *
> + * @version: Version to get
> + * @devp: Returns the TPM device
> + * Returns: 0 if OK, -ENODEV if not found
> + */
> +static int get_tpm_version(enum tpm_version version, struct udevice **devp)
> +{
> +	struct udevice *dev;
> +
> +	/*
> +	 * For now we have to probe each TPM, since the version is set up in
> +	 * of_to_plat(). We could require TPMs to declare their version when
> +	 * probed, to avoid this
> +	 */
> +	uclass_foreach_dev_probe(UCLASS_TPM, dev) {
> +		if (tpm_get_version(dev) == version) {
> +			*devp = dev;
> +			return 0;
> +		}
> +	}
> +
> +	return -ENODEV;
> +}
> +
> +/* Basic test of initing a TPM */
> +static int test_tpm_init(struct unit_test_state *uts, enum tpm_version version)
> +{
> +	struct udevice *dev;
> +
> +	/* check probe success */
> +	ut_assertok(get_tpm_version(version, &dev));
> +
> +	ut_assertok(tpm_init(dev));
> +
> +	return 0;
> +}
> +
>  static int dm_test_tpm(struct unit_test_state *uts)
> +{
> +	ut_assertok(test_tpm_init(uts, TPM_V1));
> +	ut_assertok(test_tpm_init(uts, TPM_V2));
> +
> +	return 0;
> +}
> +DM_TEST(dm_test_tpm, UT_TESTF_SCAN_FDT);
> +
> +/* Test report_state */
> +static int dm_test_tpm_report_state(struct unit_test_state *uts)
>  {
>  	struct udevice *dev;
>  	char buf[50];
>
>  	/* check probe success */
> -	ut_assertok(uclass_first_device_err(UCLASS_TPM, &dev));
> -	ut_assert(tpm_is_v2(dev));
> +	ut_assertok(get_tpm_version(TPM_V2, &dev));
>
>  	ut_assert(tpm_report_state(dev, buf, sizeof(buf)));
>  	ut_asserteq_str("init_done=0", buf);
>
> -	ut_assertok(tpm_init(dev));
> -	 /*
> -	  * tpm_auto_start will rerun tpm_init, but handles the
> -	  * -EBUSY return code internally.
> -	  */
>  	ut_assertok(tpm_auto_start(dev));
>
>  	ut_assert(tpm_report_state(dev, buf, sizeof(buf)));
> @@ -36,4 +78,4 @@ static int dm_test_tpm(struct unit_test_state *uts)
>
>  	return 0;
>  }
> -DM_TEST(dm_test_tpm, UT_TESTF_SCAN_FDT);
> +DM_TEST(dm_test_tpm_report_state, UT_TESTF_SCAN_FDT);
> --
> 2.39.2.637.g21b0678d19-goog
>

Acked-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>