diff mbox series

[v4,4/8] tpm: Add the RNG child device

Message ID 20220309122800.3940001-5-sughosh.ganu@linaro.org
State New
Headers show
Series tpm: rng: Move TPM RNG functionality to driver model | expand

Commit Message

Sughosh Ganu March 9, 2022, 12:27 p.m. UTC
The TPM device comes with the random number generator(RNG)
functionality which is built into the TPM device. Add logic to add the
RNG child device in the TPM uclass post probe callback.

The RNG device can then be used to pass a set of random bytes to the
linux kernel, need for address space randomisation through the
EFI_RNG_PROTOCOL interface.

Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
---

Changes since V3:

* Build the RNG child addition only for the u-boot proper stage using
  the CONFIG_{SPL,TPL}_BUILD guards instead of CONFIG_TPM config which
  gets included in all stages. 
* Remove the child_pre_probe callback which was starting the TPM
  device based on review from Simon.

 drivers/tpm/tpm-uclass.c | 40 ++++++++++++++++++++++++++++++++++++----
 lib/Kconfig              |  1 +
 2 files changed, 37 insertions(+), 4 deletions(-)

Comments

Simon Glass March 12, 2022, 2:43 a.m. UTC | #1
Hi Sughosh,

On Wed, 9 Mar 2022 at 05:28, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
>
> The TPM device comes with the random number generator(RNG)
> functionality which is built into the TPM device. Add logic to add the
> RNG child device in the TPM uclass post probe callback.
>
> The RNG device can then be used to pass a set of random bytes to the
> linux kernel, need for address space randomisation through the
> EFI_RNG_PROTOCOL interface.
>
> Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> ---
>
> Changes since V3:
>
> * Build the RNG child addition only for the u-boot proper stage using
>   the CONFIG_{SPL,TPL}_BUILD guards instead of CONFIG_TPM config which
>   gets included in all stages.
> * Remove the child_pre_probe callback which was starting the TPM
>   device based on review from Simon.
>
>  drivers/tpm/tpm-uclass.c | 40 ++++++++++++++++++++++++++++++++++++----
>  lib/Kconfig              |  1 +
>  2 files changed, 37 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/tpm/tpm-uclass.c b/drivers/tpm/tpm-uclass.c
> index f67fe1019b..241ed01e68 100644
> --- a/drivers/tpm/tpm-uclass.c
> +++ b/drivers/tpm/tpm-uclass.c
> @@ -11,10 +11,16 @@
>  #include <log.h>
>  #include <linux/delay.h>
>  #include <linux/unaligned/be_byteshift.h>
> +#include <tpm_api.h>
>  #include <tpm-v1.h>
>  #include <tpm-v2.h>
>  #include "tpm_internal.h"
>
> +#include <dm/lists.h>
> +
> +#define TPM_RNG1_DRV_NAME      "tpm1-rng"
> +#define TPM_RNG2_DRV_NAME      "tpm2-rng"
> +
>  int tpm_open(struct udevice *dev)
>  {
>         struct tpm_ops *ops = tpm_get_ops(dev);
> @@ -136,12 +142,38 @@ int tpm_xfer(struct udevice *dev, const uint8_t *sendbuf, size_t send_size,
>         return 0;
>  }
>
> +#if !IS_ENABLED(CONFIG_SPL_BUILD) && !IS_ENABLED(CONFIG_TPL_BUILD)

Drop that

> +static int tpm_uclass_post_probe(struct udevice *dev)
> +{
> +       int ret;
> +       const char *drv = tpm_is_v1(dev) ?
> +               TPM_RNG1_DRV_NAME : TPM_RNG2_DRV_NAME;
> +       struct udevice *child;
> +

if (CONFIG_IS_ENABLED(TPM_RNG))  {

> +       ret = device_bind_driver(dev, drv, "tpm-rng0", &child);
> +       if (ret == -ENOENT) {
> +               log_err("No driver configured for tpm-rng device\n");

That is a lot of code size for the string that will likely never
happen. You can use

> +               return 0;

You should report the error.

> +       }
> +
> +       if (ret) {
> +               log_err("Unable to bind rng driver with the tpm-rng device\n");

You might want to use

   return log_msg_ret("bind", ret)

since it doesn't add to code since unless you turn on CONFIG_LOG_ERROR_RETURN

> +               return ret;
> +       }
> +
> +       return 0;
> +}
> +#endif /* !CONFIG_SPL_BUILD && !CONFIG_TPL_BUILD */
> +
>  UCLASS_DRIVER(tpm) = {
> -       .id             = UCLASS_TPM,
> -       .name           = "tpm",
> -       .flags          = DM_UC_FLAG_SEQ_ALIAS,
> +       .id                     = UCLASS_TPM,
> +       .name                   = "tpm",
> +       .flags                  = DM_UC_FLAG_SEQ_ALIAS,
>  #if CONFIG_IS_ENABLED(OF_REAL)
> -       .post_bind      = dm_scan_fdt_dev,
> +       .post_bind              = dm_scan_fdt_dev,
> +#endif
> +#if !IS_ENABLED(CONFIG_SPL_BUILD) && !IS_ENABLED(CONFIG_TPL_BUILD)

Drop that and do it in the function as above

> +       .post_probe             = tpm_uclass_post_probe,
>  #endif
>         .per_device_auto        = sizeof(struct tpm_chip_priv),
>  };
> diff --git a/lib/Kconfig b/lib/Kconfig
> index 3c6fa99b1a..0f05c97afc 100644
> --- a/lib/Kconfig
> +++ b/lib/Kconfig
> @@ -341,6 +341,7 @@ source lib/crypt/Kconfig
>  config TPM
>         bool "Trusted Platform Module (TPM) Support"
>         depends on DM
> +       select DM_RNG

imply might be better, so boards can disable it.

>         help
>           This enables support for TPMs which can be used to provide security
>           features for your board. The TPM can be connected via LPC or I2C
> --
> 2.25.1
>

Regards,
Simon
Sughosh Ganu March 13, 2022, 11:21 a.m. UTC | #2
hi Simon,

On Sat, 12 Mar 2022 at 08:14, Simon Glass <sjg@chromium.org> wrote:
>
> Hi Sughosh,
>
> On Wed, 9 Mar 2022 at 05:28, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
> >
> > The TPM device comes with the random number generator(RNG)
> > functionality which is built into the TPM device. Add logic to add the
> > RNG child device in the TPM uclass post probe callback.
> >
> > The RNG device can then be used to pass a set of random bytes to the
> > linux kernel, need for address space randomisation through the
> > EFI_RNG_PROTOCOL interface.
> >
> > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > ---
> >
> > Changes since V3:
> >
> > * Build the RNG child addition only for the u-boot proper stage using
> >   the CONFIG_{SPL,TPL}_BUILD guards instead of CONFIG_TPM config which
> >   gets included in all stages.
> > * Remove the child_pre_probe callback which was starting the TPM
> >   device based on review from Simon.
> >
> >  drivers/tpm/tpm-uclass.c | 40 ++++++++++++++++++++++++++++++++++++----
> >  lib/Kconfig              |  1 +
> >  2 files changed, 37 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/tpm/tpm-uclass.c b/drivers/tpm/tpm-uclass.c
> > index f67fe1019b..241ed01e68 100644
> > --- a/drivers/tpm/tpm-uclass.c
> > +++ b/drivers/tpm/tpm-uclass.c
> > @@ -11,10 +11,16 @@
> >  #include <log.h>
> >  #include <linux/delay.h>
> >  #include <linux/unaligned/be_byteshift.h>
> > +#include <tpm_api.h>
> >  #include <tpm-v1.h>
> >  #include <tpm-v2.h>
> >  #include "tpm_internal.h"
> >
> > +#include <dm/lists.h>
> > +
> > +#define TPM_RNG1_DRV_NAME      "tpm1-rng"
> > +#define TPM_RNG2_DRV_NAME      "tpm2-rng"
> > +
> >  int tpm_open(struct udevice *dev)
> >  {
> >         struct tpm_ops *ops = tpm_get_ops(dev);
> > @@ -136,12 +142,38 @@ int tpm_xfer(struct udevice *dev, const uint8_t *sendbuf, size_t send_size,
> >         return 0;
> >  }
> >
> > +#if !IS_ENABLED(CONFIG_SPL_BUILD) && !IS_ENABLED(CONFIG_TPL_BUILD)
>
> Drop that

Okay. Btw, putting the check for the config symbol as you suggest
below does add some size to the SPL/TPL code. But that is theoretical,
since we currently do not enable CONFIG_{SPL,TPL}_TPM.

>
> > +static int tpm_uclass_post_probe(struct udevice *dev)
> > +{
> > +       int ret;
> > +       const char *drv = tpm_is_v1(dev) ?
> > +               TPM_RNG1_DRV_NAME : TPM_RNG2_DRV_NAME;
> > +       struct udevice *child;
> > +
>
> if (CONFIG_IS_ENABLED(TPM_RNG))  {

Okay

>
> > +       ret = device_bind_driver(dev, drv, "tpm-rng0", &child);
> > +       if (ret == -ENOENT) {
> > +               log_err("No driver configured for tpm-rng device\n");
>
> That is a lot of code size for the string that will likely never
> happen. You can use
>
> > +               return 0;
>
> You should report the error.
>
> > +       }
> > +
> > +       if (ret) {
> > +               log_err("Unable to bind rng driver with the tpm-rng device\n");
>
> You might want to use
>
>    return log_msg_ret("bind", ret)
>
> since it doesn't add to code since unless you turn on CONFIG_LOG_ERROR_RETURN

Okay.

-sughosh

>
> > +               return ret;
> > +       }
> > +
> > +       return 0;
> > +}
> > +#endif /* !CONFIG_SPL_BUILD && !CONFIG_TPL_BUILD */
> > +
> >  UCLASS_DRIVER(tpm) = {
> > -       .id             = UCLASS_TPM,
> > -       .name           = "tpm",
> > -       .flags          = DM_UC_FLAG_SEQ_ALIAS,
> > +       .id                     = UCLASS_TPM,
> > +       .name                   = "tpm",
> > +       .flags                  = DM_UC_FLAG_SEQ_ALIAS,
> >  #if CONFIG_IS_ENABLED(OF_REAL)
> > -       .post_bind      = dm_scan_fdt_dev,
> > +       .post_bind              = dm_scan_fdt_dev,
> > +#endif
> > +#if !IS_ENABLED(CONFIG_SPL_BUILD) && !IS_ENABLED(CONFIG_TPL_BUILD)
>
> Drop that and do it in the function as above
>
> > +       .post_probe             = tpm_uclass_post_probe,
> >  #endif
> >         .per_device_auto        = sizeof(struct tpm_chip_priv),
> >  };
> > diff --git a/lib/Kconfig b/lib/Kconfig
> > index 3c6fa99b1a..0f05c97afc 100644
> > --- a/lib/Kconfig
> > +++ b/lib/Kconfig
> > @@ -341,6 +341,7 @@ source lib/crypt/Kconfig
> >  config TPM
> >         bool "Trusted Platform Module (TPM) Support"
> >         depends on DM
> > +       select DM_RNG
>
> imply might be better, so boards can disable it.
>
> >         help
> >           This enables support for TPMs which can be used to provide security
> >           features for your board. The TPM can be connected via LPC or I2C
> > --
> > 2.25.1
> >
>
> Regards,
> Simon
diff mbox series

Patch

diff --git a/drivers/tpm/tpm-uclass.c b/drivers/tpm/tpm-uclass.c
index f67fe1019b..241ed01e68 100644
--- a/drivers/tpm/tpm-uclass.c
+++ b/drivers/tpm/tpm-uclass.c
@@ -11,10 +11,16 @@ 
 #include <log.h>
 #include <linux/delay.h>
 #include <linux/unaligned/be_byteshift.h>
+#include <tpm_api.h>
 #include <tpm-v1.h>
 #include <tpm-v2.h>
 #include "tpm_internal.h"
 
+#include <dm/lists.h>
+
+#define TPM_RNG1_DRV_NAME	"tpm1-rng"
+#define TPM_RNG2_DRV_NAME	"tpm2-rng"
+
 int tpm_open(struct udevice *dev)
 {
 	struct tpm_ops *ops = tpm_get_ops(dev);
@@ -136,12 +142,38 @@  int tpm_xfer(struct udevice *dev, const uint8_t *sendbuf, size_t send_size,
 	return 0;
 }
 
+#if !IS_ENABLED(CONFIG_SPL_BUILD) && !IS_ENABLED(CONFIG_TPL_BUILD)
+static int tpm_uclass_post_probe(struct udevice *dev)
+{
+	int ret;
+	const char *drv = tpm_is_v1(dev) ?
+		TPM_RNG1_DRV_NAME : TPM_RNG2_DRV_NAME;
+	struct udevice *child;
+
+	ret = device_bind_driver(dev, drv, "tpm-rng0", &child);
+	if (ret == -ENOENT) {
+		log_err("No driver configured for tpm-rng device\n");
+		return 0;
+	}
+
+	if (ret) {
+		log_err("Unable to bind rng driver with the tpm-rng device\n");
+		return ret;
+	}
+
+	return 0;
+}
+#endif /* !CONFIG_SPL_BUILD && !CONFIG_TPL_BUILD */
+
 UCLASS_DRIVER(tpm) = {
-	.id		= UCLASS_TPM,
-	.name		= "tpm",
-	.flags		= DM_UC_FLAG_SEQ_ALIAS,
+	.id			= UCLASS_TPM,
+	.name			= "tpm",
+	.flags			= DM_UC_FLAG_SEQ_ALIAS,
 #if CONFIG_IS_ENABLED(OF_REAL)
-	.post_bind	= dm_scan_fdt_dev,
+	.post_bind		= dm_scan_fdt_dev,
+#endif
+#if !IS_ENABLED(CONFIG_SPL_BUILD) && !IS_ENABLED(CONFIG_TPL_BUILD)
+	.post_probe		= tpm_uclass_post_probe,
 #endif
 	.per_device_auto	= sizeof(struct tpm_chip_priv),
 };
diff --git a/lib/Kconfig b/lib/Kconfig
index 3c6fa99b1a..0f05c97afc 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -341,6 +341,7 @@  source lib/crypt/Kconfig
 config TPM
 	bool "Trusted Platform Module (TPM) Support"
 	depends on DM
+	select DM_RNG
 	help
 	  This enables support for TPMs which can be used to provide security
 	  features for your board. The TPM can be connected via LPC or I2C