Hi Sughosh,
On Sun, 13 Mar 2022 at 08:48, 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.
Please don't do that
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
Changes since V4:
- Call the existing tpm_get_random API function from the TPM RNG driver, instead of the tpm{1,2}_get_random API's
- Introduce a new Kconfig symbol TPM_RNG and build the corresponding driver if the symbol is enabled
- Change the last parameter of the tpm_get_random API to have a data type of size_t instead of u32 to comply with the RNG driver model API
drivers/rng/Kconfig | 7 +++++++ drivers/rng/Makefile | 1 + drivers/rng/tpm_rng.c | 23 +++++++++++++++++++++++ include/tpm-v1.h | 4 ++-- include/tpm-v2.h | 4 ++-- include/tpm_api.h | 2 +- lib/Kconfig | 1 + lib/tpm-v1.c | 16 +++++++++------- lib/tpm-v2.c | 9 +++++---- lib/tpm_api.c | 16 +++++++++++----- 10 files changed, 62 insertions(+), 21 deletions(-) create mode 100644 drivers/rng/tpm_rng.c
diff --git a/drivers/rng/Kconfig b/drivers/rng/Kconfig index b1c5ab93d1..a89fa99ffa 100644 --- a/drivers/rng/Kconfig +++ b/drivers/rng/Kconfig @@ -49,4 +49,11 @@ config RNG_IPROC200 depends on DM_RNG help Enable random number generator for RPI4.
+config TPM_RNG
bool "Enable random number generator on TPM device"depends on TPMdefault yhelpEnable random number generator on TPM devices
Needs 3 lines of text so please add more detail
endif diff --git a/drivers/rng/Makefile b/drivers/rng/Makefile index 39f7ee3f03..a21f3353ea 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_RNG) += tpm_rng.o diff --git a/drivers/rng/tpm_rng.c b/drivers/rng/tpm_rng.c new file mode 100644 index 0000000000..69b41dbbf5 --- /dev/null +++ b/drivers/rng/tpm_rng.c @@ -0,0 +1,23 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/*
- Copyright (c) 2022, Linaro Limited
- */
+#include <dm.h> +#include <rng.h> +#include <tpm_api.h>
+static int rng_tpm_random_read(struct udevice *dev, void *data, size_t count) +{
return tpm_get_random(dev->parent, data, count);
dev_get_parent(dev)
Here you should check the return value and decide whether to return an error, such as -EIO
+}
+static const struct dm_rng_ops tpm_rng_ops = {
.read = rng_tpm_random_read,+};
+U_BOOT_DRIVER(tpm_rng) = {
.name = "tpm-rng",.id = UCLASS_RNG,.ops = &tpm_rng_ops,+}; 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);
I think I mentioned this last time, but you should not change these
/**
- 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/include/tpm_api.h b/include/tpm_api.h index 4d77a49719..6d9214b335 100644 --- a/include/tpm_api.h +++ b/include/tpm_api.h @@ -276,7 +276,7 @@ u32 tpm_find_key_sha1(struct udevice *dev, const u8 auth[20],
- @param count size of output buffer
- Return: 0 if OK, -ve on error
*/ -int tpm_get_random(struct udevice *dev, void *data, u32 count); +int tpm_get_random(struct udevice *dev, void *data, size_t count);
/**
- tpm_finalise_physical_presence() - Finalise physical presence
diff --git a/lib/Kconfig b/lib/Kconfig index 3c6fa99b1a..f04a9c8c79 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
imply 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 I2Cdiff --git a/lib/tpm-v1.c b/lib/tpm-v1.c index 22a769c587..4c6bc31a64 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;
return -EIO;
No please leave these alone.
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;
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;diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c index 1bf627853a..d7a23ccf7e 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;
return -EIO;
and here
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;
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;diff --git a/lib/tpm_api.c b/lib/tpm_api.c index da48058abe..8bf60cda3c 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,17 @@ u32 tpm_get_permissions(struct udevice *dev, u32 index, u32 *perm) return -ENOSYS; }
-int tpm_get_random(struct udevice *dev, void *data, u32 count) +int tpm_get_random(struct udevice *dev, void *data, size_t count) {
int ret = -ENOSYS;if (tpm_is_v1(dev))
return tpm1_get_random(dev, data, count);
ret = tpm1_get_random(dev, data, count); else if (tpm_is_v2(dev))
return -ENOSYS; /* not implemented yet */elsereturn -ENOSYS;
ret = tpm2_get_random(dev, data, count);if (ret)log_err("Failed to read random bytes\n");
I don't see this in the other functions in this file. Why add it? It just bloats the code and there is no way for the caller to suppress the message.
return ret;}
2.25.1
Regards, Simon