[PATCH 07/10] tpm: rng: Move the TPM RNG functionality to driver model

Sughosh Ganu sughosh.ganu at linaro.org
Fri Feb 25 06:47:08 CET 2022


hello Heinrich,

On Fri, 25 Feb 2022 at 00:23, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>
> On 2/24/22 19:05, Sughosh Ganu wrote:
> > Currently, the TPM random number generator(RNG) functions are defined
> > as part of the library functions under the corresponding tpm files for
> > tpmv1 and tpmv2. Move the RNG functionality under TPM RNG drivers
> > complying with the driver model.
> >
> > Also make changes to the tpm_get_random function to have it call the
> > TPM RNG driver functions instead of the library functions.
> >
> > Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org>
> > ---
> >   drivers/rng/Makefile   |  1 +
> >   drivers/rng/tpm1_rng.c | 68 ++++++++++++++++++++++++++++++++++++++++++
> >   drivers/rng/tpm2_rng.c | 68 ++++++++++++++++++++++++++++++++++++++++++
> >   lib/tpm-v1.c           | 44 ---------------------------
> >   lib/tpm-v2.c           | 44 ---------------------------
> >   lib/tpm_api.c          | 28 +++++++++++++----
> >   6 files changed, 160 insertions(+), 93 deletions(-)
> >   create mode 100644 drivers/rng/tpm1_rng.c
> >   create mode 100644 drivers/rng/tpm2_rng.c
> >
> > diff --git a/drivers/rng/Makefile b/drivers/rng/Makefile
> > index 39f7ee3f03..129cfbd006 100644
> > --- a/drivers/rng/Makefile
> > +++ b/drivers/rng/Makefile
> > @@ -10,3 +10,4 @@ obj-$(CONFIG_RNG_MSM) += msm_rng.o
> >   obj-$(CONFIG_RNG_STM32MP1) += stm32mp1_rng.o
> >   obj-$(CONFIG_RNG_ROCKCHIP) += rockchip_rng.o
> >   obj-$(CONFIG_RNG_IPROC200) += iproc_rng200.o
> > +obj-$(CONFIG_TPM) += tpm1_rng.o tpm2_rng.o
> > diff --git a/drivers/rng/tpm1_rng.c b/drivers/rng/tpm1_rng.c
> > new file mode 100644
> > index 0000000000..ddb20b008d
> > --- /dev/null
> > +++ b/drivers/rng/tpm1_rng.c
> > @@ -0,0 +1,68 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright (c) 2022, Linaro Limited
> > + */
> > +
> > +#include <common.h>
> > +#include <dm.h>
> > +#include <rng.h>
> > +#include <tpm-utils.h>
> > +#include <tpm-v1.h>
> > +
> > +#define TPM_HEADER_SIZE              10
> > +
> > +#define TPMV1_DATA_OFFSET    14
> > +
>
> Please, provide a Sphinx style function description.

Thanks for the review of this patch series. I will incorporate all
your review comments in the next version. Thanks.

-sughosh

>
> > +static int tpm1_rng_read(struct udevice *dev, void *data, size_t count)
> > +{
> > +     const u8 command[14] = {
> > +             0x0, 0xc1,              /* TPM_TAG */
> > +             0x0, 0x0, 0x0, 0xe,     /* parameter size */
> > +             0x0, 0x0, 0x0, 0x46,    /* TPM_COMMAND_CODE */
> > +     };
> > +     const size_t length_offset = TPM_HEADER_SIZE;
> > +     const size_t data_size_offset = TPM_HEADER_SIZE;
> > +     const size_t data_offset = TPMV1_DATA_OFFSET;
> > +     u8 buf[COMMAND_BUFFER_SIZE], response[COMMAND_BUFFER_SIZE];
> > +     size_t response_length = sizeof(response);
> > +     u32 data_size;
> > +     u8 *out = data;
> > +
> > +     while (count > 0) {
> > +             u32 this_bytes = min(count,
> > +                                  sizeof(response) - data_offset);
> > +             u32 err;
> > +
> > +             if (pack_byte_string(buf, sizeof(buf), "sd",
> > +                                  0, command, sizeof(command),
> > +                                  length_offset, this_bytes))
> > +                     return TPM_LIB_ERROR;
>
> Why should we return -EPERM (= TPM_LIB_ERROR)  here?
>
> How about -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;
>
> -EIO
>
> > +             if (data_size > count)
> > +                     return TPM_LIB_ERROR;
>
> -EIO
>
> > +             if (unpack_byte_string(response, response_length, "s",
> > +                                    data_offset, out, data_size))
> > +                     return TPM_LIB_ERROR;
>
> -EIO
>
> > +
> > +             count -= data_size;
> > +             out += data_size;
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> > +static const struct dm_rng_ops tpm1_rng_ops = {
> > +     .read = tpm1_rng_read,
> > +};
> > +
> > +U_BOOT_DRIVER(tpm1_rng) = {
> > +     .name           = "tpm1-rng",
> > +     .id             = UCLASS_RNG,
> > +     .ops            = &tpm1_rng_ops,
> > +};
> > diff --git a/drivers/rng/tpm2_rng.c b/drivers/rng/tpm2_rng.c
> > new file mode 100644
> > index 0000000000..f14528f751
> > --- /dev/null
> > +++ b/drivers/rng/tpm2_rng.c
> > @@ -0,0 +1,68 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright (c) 2022, Linaro Limited
> > + */
> > +
> > +#include <common.h>
> > +#include <dm.h>
> > +#include <rng.h>
> > +#include <tpm-utils.h>
> > +#include <tpm-v2.h>
> > +
> > +#define TPM_HEADER_SIZE              10
> > +
> > +#define TPMV2_DATA_OFFSET    12
> > +
>
> Please, add a Sphinx style function description.
>
> > +static int tpm2_rng_read(struct udevice *dev, void *data, size_t count)
> > +{
> > +     const u8 command_v2[10] = {
> > +             tpm_u16(TPM2_ST_NO_SESSIONS),
> > +             tpm_u32(12),
> > +             tpm_u32(TPM2_CC_GET_RANDOM),
> > +     };
> > +     u8 buf[COMMAND_BUFFER_SIZE], response[COMMAND_BUFFER_SIZE];
> > +
> > +     const size_t data_size_offset = TPM_HEADER_SIZE;
> > +     const size_t data_offset = TPMV2_DATA_OFFSET;
> > +     size_t response_length = sizeof(response);
> > +     u32 data_size;
> > +     u8 *out = data;
> > +
> > +     while (count > 0) {
> > +             u32 this_bytes = min((size_t)count,
> > +                                  sizeof(response) - data_offset);
> > +             u32 err;
> > +
> > +             if (pack_byte_string(buf, sizeof(buf), "sw",
> > +                                  0, command_v2, sizeof(command_v2),
> > +                                  sizeof(command_v2), this_bytes))
> > +                     return TPM_LIB_ERROR;
>
> -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;
>
> -EIO
>
> > +             if (data_size > this_bytes)
> > +                     return TPM_LIB_ERROR;
>
> -EIO
>
> > +             if (unpack_byte_string(response, response_length, "s",
> > +                                    data_offset, out, data_size))
> > +                     return TPM_LIB_ERROR;
>
> -EIO
>
> Best regards
>
> Heinrich
>
> > +
> > +             count -= data_size;
> > +             out += data_size;
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> > +static const struct dm_rng_ops tpm2_rng_ops = {
> > +     .read = tpm2_rng_read,
> > +};
> > +
> > +U_BOOT_DRIVER(tpm2_rng) = {
> > +     .name           = "tpm2-rng",
> > +     .id             = UCLASS_RNG,
> > +     .ops            = &tpm2_rng_ops,
> > +};
> > diff --git a/lib/tpm-v1.c b/lib/tpm-v1.c
> > index 467992e04e..71cc90a2ab 100644
> > --- a/lib/tpm-v1.c
> > +++ b/lib/tpm-v1.c
> > @@ -868,47 +868,3 @@ u32 tpm1_find_key_sha1(struct udevice *dev, const u8 auth[20],
> >   #endif /* CONFIG_TPM_LOAD_KEY_BY_SHA1 */
> >
> >   #endif /* CONFIG_TPM_AUTH_SESSIONS */
> > -
> > -u32 tpm1_get_random(struct udevice *dev, void *data, u32 count)
> > -{
> > -     const u8 command[14] = {
> > -             0x0, 0xc1,              /* TPM_TAG */
> > -             0x0, 0x0, 0x0, 0xe,     /* parameter size */
> > -             0x0, 0x0, 0x0, 0x46,    /* TPM_COMMAND_CODE */
> > -     };
> > -     const size_t length_offset = 10;
> > -     const size_t data_size_offset = 10;
> > -     const size_t data_offset = 14;
> > -     u8 buf[COMMAND_BUFFER_SIZE], response[COMMAND_BUFFER_SIZE];
> > -     size_t response_length = sizeof(response);
> > -     u32 data_size;
> > -     u8 *out = data;
> > -
> > -     while (count > 0) {
> > -             u32 this_bytes = min((size_t)count,
> > -                                  sizeof(response) - data_offset);
> > -             u32 err;
> > -
> > -             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,
> > -                                        &response_length);
> > -             if (err)
> > -                     return err;
> > -             if (unpack_byte_string(response, response_length, "d",
> > -                                    data_size_offset, &data_size))
> > -                     return TPM_LIB_ERROR;
> > -             if (data_size > count)
> > -                     return TPM_LIB_ERROR;
> > -             if (unpack_byte_string(response, response_length, "s",
> > -                                    data_offset, out, data_size))
> > -                     return TPM_LIB_ERROR;
> > -
> > -             count -= data_size;
> > -             out += data_size;
> > -     }
> > -
> > -     return 0;
> > -}
> > diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c
> > index 2f16b0007b..c1456c1974 100644
> > --- a/lib/tpm-v2.c
> > +++ b/lib/tpm-v2.c
> > @@ -562,50 +562,6 @@ 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)
> > -{
> > -     const u8 command_v2[10] = {
> > -             tpm_u16(TPM2_ST_NO_SESSIONS),
> > -             tpm_u32(12),
> > -             tpm_u32(TPM2_CC_GET_RANDOM),
> > -     };
> > -     u8 buf[COMMAND_BUFFER_SIZE], response[COMMAND_BUFFER_SIZE];
> > -
> > -     const size_t data_size_offset = 10;
> > -     const size_t data_offset = 12;
> > -     size_t response_length = sizeof(response);
> > -     u32 data_size;
> > -     u8 *out = data;
> > -
> > -     while (count > 0) {
> > -             u32 this_bytes = min((size_t)count,
> > -                                  sizeof(response) - data_offset);
> > -             u32 err;
> > -
> > -             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,
> > -                                        &response_length);
> > -             if (err)
> > -                     return err;
> > -             if (unpack_byte_string(response, response_length, "w",
> > -                                    data_size_offset, &data_size))
> > -                     return TPM_LIB_ERROR;
> > -             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;
> > -
> > -             count -= data_size;
> > -             out += data_size;
> > -     }
> > -
> > -     return 0;
> > -}
> > -
> >   u32 tpm2_write_lock(struct udevice *dev, u32 index)
> >   {
> >       u8 command_v2[COMMAND_BUFFER_SIZE] = {
> > diff --git a/lib/tpm_api.c b/lib/tpm_api.c
> > index 9dd9606fa8..de822113b0 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>
> > @@ -264,12 +265,29 @@ 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)
> >   {
> > -     if (is_tpm1(dev))
> > -             return tpm1_get_random(dev, data, count);
> > -     else if (is_tpm2(dev))
> > -             return -ENOSYS; /* not implemented yet */
> > -     else
> > +     int ret;
> > +     struct udevice *rng_dev;
> > +
> > +     if (is_tpm1(dev)) {
> > +             ret = uclass_get_device_by_driver(UCLASS_RNG,
> > +                                               DM_DRIVER_GET(tpm1_rng),
> > +                                               &rng_dev);
> > +     } else if (is_tpm2(dev)) {
> > +             ret = uclass_get_device_by_driver(UCLASS_RNG,
> > +                                               DM_DRIVER_GET(tpm2_rng),
> > +                                               &rng_dev);
> > +     } else {
> >               return -ENOSYS;
> > +     }
> > +
> > +     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 */
>


More information about the U-Boot mailing list