hello Heinrich,
On Fri, 25 Feb 2022 at 00:23, Heinrich Schuchardt <xypron.g...@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.g...@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 */
>
