[PATCH v3 3/8] tpm: rng: Add driver model interface for TPM RNG device

Simon Glass sjg at chromium.org
Wed Mar 9 03:35:38 CET 2022


Hi Sugosh,

On Fri, 4 Mar 2022 at 06:35, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
>
> The TPM device has a builtin random number generator(RNG)
> functionality. Expose the RNG functions of the TPM device to the
> driver model so that they can be used by the EFI_RNG_PROTOCOL if the
> protocol is installed.
>
> Also change the function arguments and return type of the random
> number functions to comply with the driver model api.
>
> Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org>
> ---
>
> Changes since V2:
>
> * Export the existing tpm*_get_random functions to the driver model
>   instead of moving them to the drivers/rng/ directory.
>
>  include/tpm-v1.h |  4 ++--
>  include/tpm-v2.h |  4 ++--
>  lib/tpm-v1.c     | 28 ++++++++++++++++++++--------
>  lib/tpm-v2.c     | 21 ++++++++++++++++-----
>  lib/tpm_api.c    | 23 +++++++++++++++++++----
>  5 files changed, 59 insertions(+), 21 deletions(-)
>
> diff --git a/include/tpm-v1.h b/include/tpm-v1.h
> index 33d53fb695..d2ff8b446d 100644
> --- a/include/tpm-v1.h
> +++ b/include/tpm-v1.h
> @@ -555,9 +555,9 @@ u32 tpm1_find_key_sha1(struct udevice *dev, const u8 auth[20],
>   * @param dev          TPM device
>   * @param data         output buffer for the random bytes
>   * @param count                size of output buffer
> - * Return: return code of the operation
> + * Return: 0 if OK, -ve on error
>   */
> -u32 tpm1_get_random(struct udevice *dev, void *data, u32 count);
> +int tpm1_get_random(struct udevice *dev, void *data, size_t count);
>
>  /**
>   * tpm_finalise_physical_presence() - Finalise physical presence
> diff --git a/include/tpm-v2.h b/include/tpm-v2.h
> index e79c90b939..4fb1e7a948 100644
> --- a/include/tpm-v2.h
> +++ b/include/tpm-v2.h
> @@ -619,9 +619,9 @@ u32 tpm2_pcr_setauthvalue(struct udevice *dev, const char *pw,
>   * @param data         output buffer for the random bytes
>   * @param count                size of output buffer
>   *
> - * Return: return code of the operation
> + * Return: 0 if OK, -ve on error
>   */
> -u32 tpm2_get_random(struct udevice *dev, void *data, u32 count);
> +int tpm2_get_random(struct udevice *dev, void *data, size_t count);
>
>  /**
>   * Lock data in the TPM
> diff --git a/lib/tpm-v1.c b/lib/tpm-v1.c
> index 22a769c587..57bb4a39cb 100644
> --- a/lib/tpm-v1.c
> +++ b/lib/tpm-v1.c
> @@ -9,12 +9,14 @@
>  #include <common.h>
>  #include <dm.h>
>  #include <log.h>
> -#include <asm/unaligned.h>
> -#include <u-boot/sha1.h>
> +#include <rng.h>
>  #include <tpm-common.h>
>  #include <tpm-v1.h>
>  #include "tpm-utils.h"
>
> +#include <asm/unaligned.h>
> +#include <u-boot/sha1.h>
> +
>  #ifdef CONFIG_TPM_AUTH_SESSIONS
>
>  #ifndef CONFIG_SHA1
> @@ -869,7 +871,7 @@ u32 tpm1_find_key_sha1(struct udevice *dev, const u8 auth[20],
>
>  #endif /* CONFIG_TPM_AUTH_SESSIONS */
>
> -u32 tpm1_get_random(struct udevice *dev, void *data, u32 count)
> +int tpm1_get_random(struct udevice *dev, void *data, size_t count)
>  {
>         const u8 command[14] = {
>                 0x0, 0xc1,              /* TPM_TAG */
> @@ -892,19 +894,19 @@ u32 tpm1_get_random(struct udevice *dev, void *data, u32 count)
>                 if (pack_byte_string(buf, sizeof(buf), "sd",
>                                      0, command, sizeof(command),
>                                      length_offset, this_bytes))
> -                       return TPM_LIB_ERROR;
> -               err = tpm_sendrecv_command(dev, buf, response,
> +                       return -EIO;
> +               err = tpm_sendrecv_command(dev->parent, buf, response,
>                                            &response_length);

Here it is a bit confused whether dev is the parent tpm device (as the
comment for this function says), or the random device.

>                 if (err)
>                         return err;
>                 if (unpack_byte_string(response, response_length, "d",
>                                        data_size_offset, &data_size))
> -                       return TPM_LIB_ERROR;
> +                       return -EIO;
>                 if (data_size > count)
> -                       return TPM_LIB_ERROR;
> +                       return -EIO;
>                 if (unpack_byte_string(response, response_length, "s",
>                                        data_offset, out, data_size))
> -                       return TPM_LIB_ERROR;
> +                       return -EIO;
>
>                 count -= data_size;
>                 out += data_size;
> @@ -912,3 +914,13 @@ u32 tpm1_get_random(struct udevice *dev, void *data, u32 count)
>
>         return 0;
>  }
> +
> +static const struct dm_rng_ops tpm1_rng_ops = {
> +       .read = tpm1_get_random,
> +};
> +
> +U_BOOT_DRIVER(tpm1_rng) = {
> +       .name   = "tpm1-rng",
> +       .id     = UCLASS_RNG,
> +       .ops    = &tpm1_rng_ops,
> +};

This declaration and the ops should go in the random driver. There
should be a op function in that driver which does the API call to the
TPM. Here you are duplicating this driver in two places.

Then you don't need to change the signature of tpm1_get_random().

> diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c
> index 1bf627853a..18f64f0886 100644
> --- a/lib/tpm-v2.c
> +++ b/lib/tpm-v2.c
> @@ -6,6 +6,7 @@
>
>  #include <common.h>
>  #include <dm.h>
> +#include <rng.h>
>  #include <tpm-common.h>
>  #include <tpm-v2.h>
>  #include <linux/bitops.h>
> @@ -562,7 +563,7 @@ u32 tpm2_pcr_setauthvalue(struct udevice *dev, const char *pw,
>         return tpm_sendrecv_command(dev, command_v2, NULL, NULL);
>  }
>
> -u32 tpm2_get_random(struct udevice *dev, void *data, u32 count)
> +int tpm2_get_random(struct udevice *dev, void *data, size_t count)
>  {
>         const u8 command_v2[10] = {
>                 tpm_u16(TPM2_ST_NO_SESSIONS),
> @@ -585,19 +586,19 @@ u32 tpm2_get_random(struct udevice *dev, void *data, u32 count)
>                 if (pack_byte_string(buf, sizeof(buf), "sw",
>                                      0, command_v2, sizeof(command_v2),
>                                      sizeof(command_v2), this_bytes))
> -                       return TPM_LIB_ERROR;
> -               err = tpm_sendrecv_command(dev, buf, response,
> +                       return -EIO;
> +               err = tpm_sendrecv_command(dev->parent, buf, response,
>                                            &response_length);
>                 if (err)
>                         return err;
>                 if (unpack_byte_string(response, response_length, "w",
>                                        data_size_offset, &data_size))
> -                       return TPM_LIB_ERROR;
> +                       return -EIO;
>                 if (data_size > this_bytes)
>                         return TPM_LIB_ERROR;
>                 if (unpack_byte_string(response, response_length, "s",
>                                        data_offset, out, data_size))
> -                       return TPM_LIB_ERROR;
> +                       return -EIO;
>
>                 count -= data_size;
>                 out += data_size;
> @@ -669,3 +670,13 @@ u32 tpm2_submit_command(struct udevice *dev, const u8 *sendbuf,
>  {
>         return tpm_sendrecv_command(dev, sendbuf, recvbuf, recv_size);
>  }
> +
> +static const struct dm_rng_ops tpm2_rng_ops = {
> +       .read = tpm2_get_random,
> +};
> +
> +U_BOOT_DRIVER(tpm2_rng) = {
> +       .name   = "tpm2-rng",
> +       .id     = UCLASS_RNG,
> +       .ops    = &tpm2_rng_ops,
> +};

See above.

> diff --git a/lib/tpm_api.c b/lib/tpm_api.c
> index da48058abe..3584fda98c 100644
> --- a/lib/tpm_api.c
> +++ b/lib/tpm_api.c
> @@ -6,6 +6,7 @@
>  #include <common.h>
>  #include <dm.h>
>  #include <log.h>
> +#include <rng.h>
>  #include <tpm_api.h>
>  #include <tpm-v1.h>
>  #include <tpm-v2.h>
> @@ -265,12 +266,26 @@ u32 tpm_get_permissions(struct udevice *dev, u32 index, u32 *perm)
>                 return -ENOSYS;
>  }
>
> +#if CONFIG_IS_ENABLED(DM_RNG)
>  int tpm_get_random(struct udevice *dev, void *data, u32 count)
>  {
> +       int ret = -ENOSYS;
> +       struct udevice *rng_dev;
> +
>         if (tpm_is_v1(dev))
> -               return tpm1_get_random(dev, data, count);
> +               ret = uclass_get_device_by_driver(UCLASS_RNG,
> +                                                 DM_DRIVER_GET(tpm1_rng),
> +                                                 &rng_dev);

Er, tpm_get_random() should take a tpm device. The random device
should be handled by the caller, which should call
tpm_get_random(rand_dev->parent...


>         else if (tpm_is_v2(dev))
> -               return -ENOSYS; /* not implemented yet */
> -       else
> -               return -ENOSYS;
> +               ret = uclass_get_device_by_driver(UCLASS_RNG,
> +                                                 DM_DRIVER_GET(tpm1_rng),
> +                                                 &rng_dev);
> +
> +       if (ret) {
> +               log_err("Getting tpm rng device failed\n");
> +               return ret;
> +       }
> +
> +       return dm_rng_read(rng_dev, data, (size_t)count);
>  }
> +#endif /* DM_RNG */
> --
> 2.25.1
>

Regards,
Simon


More information about the U-Boot mailing list