diff mbox series

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

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

Commit Message

Sughosh Ganu March 4, 2022, 1:34 p.m. UTC
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@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(-)

Comments

Simon Glass March 9, 2022, 2:35 a.m. UTC | #1
Hi Sugosh,

On Fri, 4 Mar 2022 at 06:35, Sughosh Ganu <sughosh.ganu@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@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
Sughosh Ganu March 9, 2022, 6 a.m. UTC | #2
hi Simon,

Thanks for looking into this. I now have a fair idea of the structure
that you are looking for this interface.

On Wed, 9 Mar 2022 at 08:05, Simon Glass <sjg@chromium.org> wrote:
>
> Hi Sugosh,
>
> On Fri, 4 Mar 2022 at 06:35, Sughosh Ganu <sughosh.ganu@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@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.

Yes, since this function is being exported as a rng uclass's read
callback, the first argument to this function will indeed be the rng
device. But this can be fixed with the suggestion that you have posted
below.

>
> >                 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.

Actually, I am not duplicating the driver, since I removed the driver
declaration from under drivers/rng/. But I will re-introduce that and
put an API call to tpm like you are suggesting.

>
> 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...

Okay. I will make the changes as per your suggestion. Thanks for the
review of the patch.

-sughosh

>
>
> >         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
Sughosh Ganu March 9, 2022, 11:18 a.m. UTC | #3
hi Simon,

On Wed, 9 Mar 2022 at 11:30, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
>
> hi Simon,
>
> Thanks for looking into this. I now have a fair idea of the structure
> that you are looking for this interface.
>
> On Wed, 9 Mar 2022 at 08:05, Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Sugosh,
> >
> > On Fri, 4 Mar 2022 at 06:35, Sughosh Ganu <sughosh.ganu@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@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.

<snip>

> > > 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...
>
> Okay. I will make the changes as per your suggestion. Thanks for the
> review of the patch.

Having had a relook at this, the tpm_get_random is indeed getting the
TPM device. Which is why the call to tpm_is_v1 is being called with
the same 'dev' argument. So I believe this function is currently as
per what you are looking for. Getting the TPM device as the first
argument.

-sughosh
Simon Glass March 12, 2022, 5:59 p.m. UTC | #4
Hi Sughosh.

On Wed, 9 Mar 2022 at 04:18, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
>
> hi Simon,
>
> On Wed, 9 Mar 2022 at 11:30, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
> >
> > hi Simon,
> >
> > Thanks for looking into this. I now have a fair idea of the structure
> > that you are looking for this interface.
> >
> > On Wed, 9 Mar 2022 at 08:05, Simon Glass <sjg@chromium.org> wrote:
> > >
> > > Hi Sugosh,
> > >
> > > On Fri, 4 Mar 2022 at 06:35, Sughosh Ganu <sughosh.ganu@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@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.
>
> <snip>
>
> > > > 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...
> >
> > Okay. I will make the changes as per your suggestion. Thanks for the
> > review of the patch.
>
> Having had a relook at this, the tpm_get_random is indeed getting the
> TPM device. Which is why the call to tpm_is_v1 is being called with
> the same 'dev' argument. So I believe this function is currently as
> per what you are looking for. Getting the TPM device as the first
> argument.

OK.

Regards,
Simon
diff mbox series

Patch

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);
 		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,
+};
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,
+};
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);
 	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 */