[PATCH v4 0/8] tpm: rng: Move TPM RNG functionality to driver model
The TPM device provides the random number generator(RNG) functionality, whereby sending a command to the TPM device results in the TPM device responding with random bytes.
There was a discussion on the mailing list earlier[1], where it was explained that platforms with a TPM device can install the EFI_RNG_PROTOCOL for getting the random bytes instead of populating the dtb with the kaslr-seed property. That would make it possible to measure the dtb.
This patchset moves the already existing functions for getting random bytes from the TPM device to drivers complying with the RNG uclass. This is done since the EFI_RNG_PROTOCOL's get_rng routine uses the RNG uclass's dm_rng_read api to get the random bytes.
The TPM uclass driver adds the RNG child device as part of it's post_probe function. The TPM uclass driver's child_pre_probe function initialises the TPM parent device for use -- this enables the RNG child device to be used subsequently.
Some additional changes have also been made to facilitate the use of the RNG devices, including extending the 'rng' command to take the RNG device as one of the command-line parameters.
This series depends on a patch[2] from Simon Glass for moving the TPM device version detection functions to the tpm_api.h header as static inline functions. [1] - https://lore.kernel.org/u-boot/20220103120738.47835-1-ilias.apalodimas@linar... [2] - https://lore.kernel.org/u-boot/20220301001125.1554442-2-sjg@chromium.org/T/#...
Changes since V3:
* Move back the driver model interface for the TPM RNG devices under drivers/rng/ directory. * Add a rng read function for the tpm devices which calls the tpm*_get_random API's. * Pass the TPM device pointer to the tpm*_get_random API's from the TPM RNG drivers as suggested by Simon. * Build the RNG child addition only for the u-boot proper stage using the CONFIG_{SPL,TPL}_BUILD guards instead of CONFIG_TPM config which gets included in all stages. * Remove the child_pre_probe callback which was starting the TPM device based on review from Simon.
Sughosh Ganu (8): tpm: rng: Change tpm_get_random to return an int tpm: Fix the return type of tpm_startup tpm: rng: Add driver model interface for TPM RNG device tpm: Add the RNG child device qemu: arm: Remove platform specific function to get RNG device cmd: rng: Add support for selecting RNG device doc: rng: Add documentation for the rng command test: rng: Add a UT testcase for the rng command
board/emulation/qemu-arm/qemu-arm.c | 42 ----------------------------- cmd/Kconfig | 1 + cmd/rng.c | 31 +++++++++++++++------ doc/usage/index.rst | 1 + doc/usage/rng.rst | 25 +++++++++++++++++ drivers/rng/Makefile | 2 ++ drivers/rng/tpm1_rng.c | 24 +++++++++++++++++ drivers/rng/tpm2_rng.c | 23 ++++++++++++++++ drivers/tpm/tpm-uclass.c | 40 ++++++++++++++++++++++++--- include/tpm-v1.h | 4 +-- include/tpm-v2.h | 4 +-- include/tpm_api.h | 6 ++--- lib/Kconfig | 1 + lib/tpm-v1.c | 16 ++++++----- lib/tpm-v2.c | 9 ++++--- lib/tpm_api.c | 28 ++++++++++++++----- test/dm/rng.c | 29 ++++++++++++++++++++ 17 files changed, 208 insertions(+), 78 deletions(-) create mode 100644 doc/usage/rng.rst create mode 100644 drivers/rng/tpm1_rng.c create mode 100644 drivers/rng/tpm2_rng.c
The tpm random number generation functionality will be moved to the driver model. With that, the tpm_get_random function will call the common driver model api instead of separate functions for tpmv1 and tpmv2. Return an int instead of a u32 to comply with the return value of the driver model function.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org Reviewed-by: Heinrich Schuchardt xypron.glpk@gmx.de Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org Reviewed-by: Simon Glass sjg@chromium.org ---
Changes since V3: None
include/tpm_api.h | 4 ++-- lib/tpm_api.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/tpm_api.h b/include/tpm_api.h index 11aa14eb79..249a966942 100644 --- a/include/tpm_api.h +++ b/include/tpm_api.h @@ -274,9 +274,9 @@ u32 tpm_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 tpm_get_random(struct udevice *dev, void *data, u32 count); +int tpm_get_random(struct udevice *dev, void *data, u32 count);
/** * tpm_finalise_physical_presence() - Finalise physical presence diff --git a/lib/tpm_api.c b/lib/tpm_api.c index 4ac4612c81..7d26ea2c3a 100644 --- a/lib/tpm_api.c +++ b/lib/tpm_api.c @@ -264,7 +264,7 @@ u32 tpm_get_permissions(struct udevice *dev, u32 index, u32 *perm) return -ENOSYS; }
-u32 tpm_get_random(struct udevice *dev, void *data, u32 count) +int tpm_get_random(struct udevice *dev, void *data, u32 count) { if (tpm_is_v1(dev)) return tpm1_get_random(dev, data, count);
The tpm_startup function returns negative values for error conditions. Fix the return type of the function to a signed int instead of a u32.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org Reviewed-by: Simon Glass sjg@chromium.org ---
Changes since V3: None
include/tpm_api.h | 2 +- lib/tpm_api.c | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/include/tpm_api.h b/include/tpm_api.h index 249a966942..4d77a49719 100644 --- a/include/tpm_api.h +++ b/include/tpm_api.h @@ -18,7 +18,7 @@ * @param mode TPM startup mode * Return: return code of the operation */ -u32 tpm_startup(struct udevice *dev, enum tpm_startup_type mode); +int tpm_startup(struct udevice *dev, enum tpm_startup_type mode);
/** * Issue a TPM_SelfTestFull command. diff --git a/lib/tpm_api.c b/lib/tpm_api.c index 7d26ea2c3a..da48058abe 100644 --- a/lib/tpm_api.c +++ b/lib/tpm_api.c @@ -11,7 +11,8 @@ #include <tpm-v2.h> #include <tpm_api.h>
-u32 tpm_startup(struct udevice *dev, enum tpm_startup_type mode) + +int tpm_startup(struct udevice *dev, enum tpm_startup_type mode) { if (tpm_is_v1(dev)) { return tpm1_startup(dev, mode);
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 V3:
* Move back the driver model interface for the TPM RNG devices under drivers/rng/ directory. * Add a rng read function for the tpm devices which calls the tpm*_get_random API's. * Pass the TPM device pointer to the tpm*_get_random API's from the TPM RNG drivers as suggested by Simon.
drivers/rng/Makefile | 2 ++ drivers/rng/tpm1_rng.c | 24 ++++++++++++++++++++++++ drivers/rng/tpm2_rng.c | 23 +++++++++++++++++++++++ include/tpm-v1.h | 4 ++-- include/tpm-v2.h | 4 ++-- lib/tpm-v1.c | 16 +++++++++------- lib/tpm-v2.c | 9 +++++---- lib/tpm_api.c | 23 +++++++++++++++++++---- 8 files changed, 86 insertions(+), 19 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..c91e4e1701 100644 --- a/drivers/rng/Makefile +++ b/drivers/rng/Makefile @@ -10,3 +10,5 @@ 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_V1) += tpm1_rng.o +obj-$(CONFIG_TPM_V2) += tpm2_rng.o diff --git a/drivers/rng/tpm1_rng.c b/drivers/rng/tpm1_rng.c new file mode 100644 index 0000000000..b827e2d1c1 --- /dev/null +++ b/drivers/rng/tpm1_rng.c @@ -0,0 +1,24 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (c) 2022, Linaro Limited + */ + +#include <dm.h> +#include <rng.h> +#include <tpm-v1.h> + +static int rng_tpm1_random_read(struct udevice *dev, void *data, size_t count) +{ + return tpm1_get_random(dev->parent, data, count); +} + +static const struct dm_rng_ops tpm1_rng_ops = { + .read = rng_tpm1_random_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..bd1fd539f0 --- /dev/null +++ b/drivers/rng/tpm2_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-v2.h> + +static int rng_tpm2_random_read(struct udevice *dev, void *data, size_t count) +{ + return tpm2_get_random(dev->parent, data, count); +} + +static const struct dm_rng_ops tpm2_rng_ops = { + .read = rng_tpm2_random_read, +}; + +U_BOOT_DRIVER(tpm2_rng) = { + .name = "tpm2-rng", + .id = UCLASS_RNG, + .ops = &tpm2_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);
/** * 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..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; 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; 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..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 */
Hi Sughosh,
On Wed, 9 Mar 2022 at 05:28, 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 V3:
- Move back the driver model interface for the TPM RNG devices under drivers/rng/ directory.
- Add a rng read function for the tpm devices which calls the tpm*_get_random API's.
- Pass the TPM device pointer to the tpm*_get_random API's from the TPM RNG drivers as suggested by Simon.
drivers/rng/Makefile | 2 ++ drivers/rng/tpm1_rng.c | 24 ++++++++++++++++++++++++ drivers/rng/tpm2_rng.c | 23 +++++++++++++++++++++++ include/tpm-v1.h | 4 ++-- include/tpm-v2.h | 4 ++-- lib/tpm-v1.c | 16 +++++++++------- lib/tpm-v2.c | 9 +++++---- lib/tpm_api.c | 23 +++++++++++++++++++---- 8 files changed, 86 insertions(+), 19 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..c91e4e1701 100644 --- a/drivers/rng/Makefile +++ b/drivers/rng/Makefile @@ -10,3 +10,5 @@ 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_V1) += tpm1_rng.o +obj-$(CONFIG_TPM_V2) += tpm2_rng.o diff --git a/drivers/rng/tpm1_rng.c b/drivers/rng/tpm1_rng.c new file mode 100644 index 0000000000..b827e2d1c1 --- /dev/null +++ b/drivers/rng/tpm1_rng.c @@ -0,0 +1,24 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/*
- Copyright (c) 2022, Linaro Limited
- */
+#include <dm.h> +#include <rng.h> +#include <tpm-v1.h>
+static int rng_tpm1_random_read(struct udevice *dev, void *data, size_t count) +{
return tpm1_get_random(dev->parent, data, count);+}
+static const struct dm_rng_ops tpm1_rng_ops = {
.read = rng_tpm1_random_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..bd1fd539f0 --- /dev/null +++ b/drivers/rng/tpm2_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-v2.h>
+static int rng_tpm2_random_read(struct udevice *dev, void *data, size_t count) +{
return tpm2_get_random(dev->parent, data, count);+}
+static const struct dm_rng_ops tpm2_rng_ops = {
.read = rng_tpm2_random_read,+};
+U_BOOT_DRIVER(tpm2_rng) = {
.name = "tpm2-rng",.id = UCLASS_RNG,.ops = &tpm2_rng_ops,+};
Can't you just have one driver and have a tpm_get_random() function in tpm_api.h ?
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);
Do you need to change these? It would be better if the caller dealt with this.
/**
- Lock data in the TPM
diff --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; 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; 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..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)
This is the function you should call
{
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);
What is going on here? This code should stay the same. Why would you go off and hunt for the device?
else if (tpm_is_v2(dev))
return -ENOSYS; /* not implemented yet */
It is implemented above
elsereturn -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);
?? Don't you want to call the TPM to get the random number? I am just lost.
}
+#endif /* DM_RNG */
2.25.1
Regards, Simon
hi Simon,
On Sat, 12 Mar 2022 at 08:14, Simon Glass sjg@chromium.org wrote:
Hi Sughosh,
On Wed, 9 Mar 2022 at 05:28, 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 V3:
- Move back the driver model interface for the TPM RNG devices under drivers/rng/ directory.
- Add a rng read function for the tpm devices which calls the tpm*_get_random API's.
- Pass the TPM device pointer to the tpm*_get_random API's from the TPM RNG drivers as suggested by Simon.
drivers/rng/Makefile | 2 ++ drivers/rng/tpm1_rng.c | 24 ++++++++++++++++++++++++ drivers/rng/tpm2_rng.c | 23 +++++++++++++++++++++++ include/tpm-v1.h | 4 ++-- include/tpm-v2.h | 4 ++-- lib/tpm-v1.c | 16 +++++++++------- lib/tpm-v2.c | 9 +++++---- lib/tpm_api.c | 23 +++++++++++++++++++---- 8 files changed, 86 insertions(+), 19 deletions(-) create mode 100644 drivers/rng/tpm1_rng.c create mode 100644 drivers/rng/tpm2_rng.c
<snip>
+static int rng_tpm2_random_read(struct udevice *dev, void *data, size_t count) +{
return tpm2_get_random(dev->parent, data, count);+}
+static const struct dm_rng_ops tpm2_rng_ops = {
.read = rng_tpm2_random_read,+};
+U_BOOT_DRIVER(tpm2_rng) = {
.name = "tpm2-rng",.id = UCLASS_RNG,.ops = &tpm2_rng_ops,+};
Can't you just have one driver and have a tpm_get_random() function in tpm_api.h ?
With a call to tpm_get_random we can, yes.
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);
Do you need to change these? It would be better if the caller dealt with this.
I am putting these functions in line with the RNG uclass interface which has the count parameter as a size_t data type.
/**
- Lock data in the TPM
diff --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; 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; 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..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)
This is the function you should call
My thinking was that the TPM drivers would use the tpm{1,2}_get_random functions directly, while tpm_get_random would be a generic API which could be called from any other module. This API would then call the driver model interface(dm_rng_read). But I see that you want this function to be called from the tpm rng driver. I have made the change in my v5.
{
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);What is going on here? This code should stay the same. Why would you go off and hunt for the device?
else if (tpm_is_v2(dev))
return -ENOSYS; /* not implemented yet */It is implemented above
Yeah I know :). This is existing code which, for some reason, was not calling the tpm2_get_random function. I am fixing this now by removing this line.
elsereturn -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);?? Don't you want to call the TPM to get the random number? I am just lost.
Please see the explanation above.
-sughosh
The TPM device comes with the random number generator(RNG) functionality which is built into the TPM device. Add logic to add the RNG child device in the TPM uclass post probe callback.
The RNG device can then be used to pass a set of random bytes to the linux kernel, need for address space randomisation through the EFI_RNG_PROTOCOL interface.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org ---
Changes since V3:
* Build the RNG child addition only for the u-boot proper stage using the CONFIG_{SPL,TPL}_BUILD guards instead of CONFIG_TPM config which gets included in all stages. * Remove the child_pre_probe callback which was starting the TPM device based on review from Simon.
drivers/tpm/tpm-uclass.c | 40 ++++++++++++++++++++++++++++++++++++---- lib/Kconfig | 1 + 2 files changed, 37 insertions(+), 4 deletions(-)
diff --git a/drivers/tpm/tpm-uclass.c b/drivers/tpm/tpm-uclass.c index f67fe1019b..241ed01e68 100644 --- a/drivers/tpm/tpm-uclass.c +++ b/drivers/tpm/tpm-uclass.c @@ -11,10 +11,16 @@ #include <log.h> #include <linux/delay.h> #include <linux/unaligned/be_byteshift.h> +#include <tpm_api.h> #include <tpm-v1.h> #include <tpm-v2.h> #include "tpm_internal.h"
+#include <dm/lists.h> + +#define TPM_RNG1_DRV_NAME "tpm1-rng" +#define TPM_RNG2_DRV_NAME "tpm2-rng" + int tpm_open(struct udevice *dev) { struct tpm_ops *ops = tpm_get_ops(dev); @@ -136,12 +142,38 @@ int tpm_xfer(struct udevice *dev, const uint8_t *sendbuf, size_t send_size, return 0; }
+#if !IS_ENABLED(CONFIG_SPL_BUILD) && !IS_ENABLED(CONFIG_TPL_BUILD) +static int tpm_uclass_post_probe(struct udevice *dev) +{ + int ret; + const char *drv = tpm_is_v1(dev) ? + TPM_RNG1_DRV_NAME : TPM_RNG2_DRV_NAME; + struct udevice *child; + + ret = device_bind_driver(dev, drv, "tpm-rng0", &child); + if (ret == -ENOENT) { + log_err("No driver configured for tpm-rng device\n"); + return 0; + } + + if (ret) { + log_err("Unable to bind rng driver with the tpm-rng device\n"); + return ret; + } + + return 0; +} +#endif /* !CONFIG_SPL_BUILD && !CONFIG_TPL_BUILD */ + UCLASS_DRIVER(tpm) = { - .id = UCLASS_TPM, - .name = "tpm", - .flags = DM_UC_FLAG_SEQ_ALIAS, + .id = UCLASS_TPM, + .name = "tpm", + .flags = DM_UC_FLAG_SEQ_ALIAS, #if CONFIG_IS_ENABLED(OF_REAL) - .post_bind = dm_scan_fdt_dev, + .post_bind = dm_scan_fdt_dev, +#endif +#if !IS_ENABLED(CONFIG_SPL_BUILD) && !IS_ENABLED(CONFIG_TPL_BUILD) + .post_probe = tpm_uclass_post_probe, #endif .per_device_auto = sizeof(struct tpm_chip_priv), }; diff --git a/lib/Kconfig b/lib/Kconfig index 3c6fa99b1a..0f05c97afc 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 + select 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 I2C
Hi Sughosh,
On Wed, 9 Mar 2022 at 05:28, Sughosh Ganu sughosh.ganu@linaro.org wrote:
The TPM device comes with the random number generator(RNG) functionality which is built into the TPM device. Add logic to add the RNG child device in the TPM uclass post probe callback.
The RNG device can then be used to pass a set of random bytes to the linux kernel, need for address space randomisation through the EFI_RNG_PROTOCOL interface.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
Changes since V3:
- Build the RNG child addition only for the u-boot proper stage using the CONFIG_{SPL,TPL}_BUILD guards instead of CONFIG_TPM config which gets included in all stages.
- Remove the child_pre_probe callback which was starting the TPM device based on review from Simon.
drivers/tpm/tpm-uclass.c | 40 ++++++++++++++++++++++++++++++++++++---- lib/Kconfig | 1 + 2 files changed, 37 insertions(+), 4 deletions(-)
diff --git a/drivers/tpm/tpm-uclass.c b/drivers/tpm/tpm-uclass.c index f67fe1019b..241ed01e68 100644 --- a/drivers/tpm/tpm-uclass.c +++ b/drivers/tpm/tpm-uclass.c @@ -11,10 +11,16 @@ #include <log.h> #include <linux/delay.h> #include <linux/unaligned/be_byteshift.h> +#include <tpm_api.h> #include <tpm-v1.h> #include <tpm-v2.h> #include "tpm_internal.h"
+#include <dm/lists.h>
+#define TPM_RNG1_DRV_NAME "tpm1-rng" +#define TPM_RNG2_DRV_NAME "tpm2-rng"
int tpm_open(struct udevice *dev) { struct tpm_ops *ops = tpm_get_ops(dev); @@ -136,12 +142,38 @@ int tpm_xfer(struct udevice *dev, const uint8_t *sendbuf, size_t send_size, return 0; }
+#if !IS_ENABLED(CONFIG_SPL_BUILD) && !IS_ENABLED(CONFIG_TPL_BUILD)
Drop that
+static int tpm_uclass_post_probe(struct udevice *dev) +{
int ret;const char *drv = tpm_is_v1(dev) ?TPM_RNG1_DRV_NAME : TPM_RNG2_DRV_NAME;struct udevice *child;
if (CONFIG_IS_ENABLED(TPM_RNG)) {
ret = device_bind_driver(dev, drv, "tpm-rng0", &child);if (ret == -ENOENT) {log_err("No driver configured for tpm-rng device\n");
That is a lot of code size for the string that will likely never happen. You can use
return 0;
You should report the error.
}if (ret) {log_err("Unable to bind rng driver with the tpm-rng device\n");
You might want to use
return log_msg_ret("bind", ret)
since it doesn't add to code since unless you turn on CONFIG_LOG_ERROR_RETURN
return ret;}return 0;+} +#endif /* !CONFIG_SPL_BUILD && !CONFIG_TPL_BUILD */
UCLASS_DRIVER(tpm) = {
.id = UCLASS_TPM,.name = "tpm",.flags = DM_UC_FLAG_SEQ_ALIAS,
.id = UCLASS_TPM,.name = "tpm",.flags = DM_UC_FLAG_SEQ_ALIAS,#if CONFIG_IS_ENABLED(OF_REAL)
.post_bind = dm_scan_fdt_dev,
.post_bind = dm_scan_fdt_dev,+#endif +#if !IS_ENABLED(CONFIG_SPL_BUILD) && !IS_ENABLED(CONFIG_TPL_BUILD)
Drop that and do it in the function as above
.post_probe = tpm_uclass_post_probe,#endif .per_device_auto = sizeof(struct tpm_chip_priv), }; diff --git a/lib/Kconfig b/lib/Kconfig index 3c6fa99b1a..0f05c97afc 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
select DM_RNG
imply might be better, so boards can disable it.
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 I2C-- 2.25.1
Regards, Simon
hi Simon,
On Sat, 12 Mar 2022 at 08:14, Simon Glass sjg@chromium.org wrote:
Hi Sughosh,
On Wed, 9 Mar 2022 at 05:28, Sughosh Ganu sughosh.ganu@linaro.org wrote:
The TPM device comes with the random number generator(RNG) functionality which is built into the TPM device. Add logic to add the RNG child device in the TPM uclass post probe callback.
The RNG device can then be used to pass a set of random bytes to the linux kernel, need for address space randomisation through the EFI_RNG_PROTOCOL interface.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
Changes since V3:
- Build the RNG child addition only for the u-boot proper stage using the CONFIG_{SPL,TPL}_BUILD guards instead of CONFIG_TPM config which gets included in all stages.
- Remove the child_pre_probe callback which was starting the TPM device based on review from Simon.
drivers/tpm/tpm-uclass.c | 40 ++++++++++++++++++++++++++++++++++++---- lib/Kconfig | 1 + 2 files changed, 37 insertions(+), 4 deletions(-)
diff --git a/drivers/tpm/tpm-uclass.c b/drivers/tpm/tpm-uclass.c index f67fe1019b..241ed01e68 100644 --- a/drivers/tpm/tpm-uclass.c +++ b/drivers/tpm/tpm-uclass.c @@ -11,10 +11,16 @@ #include <log.h> #include <linux/delay.h> #include <linux/unaligned/be_byteshift.h> +#include <tpm_api.h> #include <tpm-v1.h> #include <tpm-v2.h> #include "tpm_internal.h"
+#include <dm/lists.h>
+#define TPM_RNG1_DRV_NAME "tpm1-rng" +#define TPM_RNG2_DRV_NAME "tpm2-rng"
int tpm_open(struct udevice *dev) { struct tpm_ops *ops = tpm_get_ops(dev); @@ -136,12 +142,38 @@ int tpm_xfer(struct udevice *dev, const uint8_t *sendbuf, size_t send_size, return 0; }
+#if !IS_ENABLED(CONFIG_SPL_BUILD) && !IS_ENABLED(CONFIG_TPL_BUILD)
Drop that
Okay. Btw, putting the check for the config symbol as you suggest below does add some size to the SPL/TPL code. But that is theoretical, since we currently do not enable CONFIG_{SPL,TPL}_TPM.
+static int tpm_uclass_post_probe(struct udevice *dev) +{
int ret;const char *drv = tpm_is_v1(dev) ?TPM_RNG1_DRV_NAME : TPM_RNG2_DRV_NAME;struct udevice *child;if (CONFIG_IS_ENABLED(TPM_RNG)) {
Okay
ret = device_bind_driver(dev, drv, "tpm-rng0", &child);if (ret == -ENOENT) {log_err("No driver configured for tpm-rng device\n");That is a lot of code size for the string that will likely never happen. You can use
return 0;You should report the error.
}if (ret) {log_err("Unable to bind rng driver with the tpm-rng device\n");You might want to use
return log_msg_ret("bind", ret)
since it doesn't add to code since unless you turn on CONFIG_LOG_ERROR_RETURN
Okay.
-sughosh
return ret;}return 0;+} +#endif /* !CONFIG_SPL_BUILD && !CONFIG_TPL_BUILD */
UCLASS_DRIVER(tpm) = {
.id = UCLASS_TPM,.name = "tpm",.flags = DM_UC_FLAG_SEQ_ALIAS,
.id = UCLASS_TPM,.name = "tpm",.flags = DM_UC_FLAG_SEQ_ALIAS,#if CONFIG_IS_ENABLED(OF_REAL)
.post_bind = dm_scan_fdt_dev,
.post_bind = dm_scan_fdt_dev,+#endif +#if !IS_ENABLED(CONFIG_SPL_BUILD) && !IS_ENABLED(CONFIG_TPL_BUILD)
Drop that and do it in the function as above
.post_probe = tpm_uclass_post_probe,#endif .per_device_auto = sizeof(struct tpm_chip_priv), }; diff --git a/lib/Kconfig b/lib/Kconfig index 3c6fa99b1a..0f05c97afc 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
select DM_RNGimply might be better, so boards can disable it.
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 I2C-- 2.25.1
Regards, Simon
The Qemu platform has a function defined to get the random number generator(RNG) device. However, the RNG device can be obtained simply by searching for a device belonging to the RNG uclass. Remove the superfluous platform function defined for the Qemu platform for getting the RNG device.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org Tested-by: Heinrich Schuchardt xypron.glpk@gmx.de Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org Reviewed-by: Simon Glass sjg@chromium.org ---
Changes since V3: None
board/emulation/qemu-arm/qemu-arm.c | 42 ----------------------------- 1 file changed, 42 deletions(-)
diff --git a/board/emulation/qemu-arm/qemu-arm.c b/board/emulation/qemu-arm/qemu-arm.c index 16d5a97167..c9e886e44a 100644 --- a/board/emulation/qemu-arm/qemu-arm.c +++ b/board/emulation/qemu-arm/qemu-arm.c @@ -107,48 +107,6 @@ void enable_caches(void) dcache_enable(); }
-#if defined(CONFIG_EFI_RNG_PROTOCOL) -#include <efi_loader.h> -#include <efi_rng.h> - -#include <dm/device-internal.h> - -efi_status_t platform_get_rng_device(struct udevice **dev) -{ - int ret; - efi_status_t status = EFI_DEVICE_ERROR; - struct udevice *bus, *devp; - - for (uclass_first_device(UCLASS_VIRTIO, &bus); bus; - uclass_next_device(&bus)) { - for (device_find_first_child(bus, &devp); devp; - device_find_next_child(&devp)) { - if (device_get_uclass_id(devp) == UCLASS_RNG) { - *dev = devp; - status = EFI_SUCCESS; - break; - } - } - } - - if (status != EFI_SUCCESS) { - debug("No rng device found\n"); - return EFI_DEVICE_ERROR; - } - - if (*dev) { - ret = device_probe(*dev); - if (ret) - return EFI_DEVICE_ERROR; - } else { - debug("Couldn't get child device\n"); - return EFI_DEVICE_ERROR; - } - - return EFI_SUCCESS; -} -#endif /* CONFIG_EFI_RNG_PROTOCOL */ - #ifdef CONFIG_ARM64 #define __W "w" #else
The 'rng' u-boot command is used for printing a select number of random bytes on the console. Currently, the RNG device from which the random bytes are read is fixed. However, a platform can have multiple RNG devices, one example being qemu, which has a virtio RNG device and the RNG pseudo device through the TPM chip.
Extend the 'rng' command so that the user can provide the RNG device number from which the random bytes are to be read. This will be the device index under the RNG uclass.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org Tested-by: Heinrich Schuchardt xypron.glpk@gmx.de Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org ---
Changes since V3: None
cmd/rng.c | 31 +++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-)
diff --git a/cmd/rng.c b/cmd/rng.c index 1ad5a096c0..bb89cfa784 100644 --- a/cmd/rng.c +++ b/cmd/rng.c @@ -13,19 +13,34 @@
static int do_rng(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { - size_t n = 0x40; + size_t n; struct udevice *dev; void *buf; + int devnum; int ret = CMD_RET_SUCCESS;
- if (uclass_get_device(UCLASS_RNG, 0, &dev) || !dev) { + switch (argc) { + case 1: + devnum = 0; + n = 0x40; + break; + case 2: + devnum = hextoul(argv[1], NULL); + n = 0x40; + break; + case 3: + devnum = hextoul(argv[1], NULL); + n = hextoul(argv[2], NULL); + break; + default: + return CMD_RET_USAGE; + } + + if (uclass_get_device(UCLASS_RNG, devnum, &dev) || !dev) { printf("No RNG device\n"); return CMD_RET_FAILURE; }
- if (argc >= 2) - n = hextoul(argv[1], NULL); - buf = malloc(n); if (!buf) { printf("Out of memory\n"); @@ -46,12 +61,12 @@ static int do_rng(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
#ifdef CONFIG_SYS_LONGHELP static char rng_help_text[] = - "[n]\n" - " - print n random bytes\n"; + "[dev [n]]\n" + " - print n random bytes read from dev\n"; #endif
U_BOOT_CMD( - rng, 2, 0, do_rng, + rng, 3, 0, do_rng, "print bytes from the hardware random number generator", rng_help_text );
Add a usage document for the 'rng' u-boot command.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org Reviewed-by: Simon Glass sjg@chromium.org ---
Changes since V3: None
doc/usage/index.rst | 1 + doc/usage/rng.rst | 25 +++++++++++++++++++++++++ 2 files changed, 26 insertions(+) create mode 100644 doc/usage/rng.rst
diff --git a/doc/usage/index.rst b/doc/usage/index.rst index 0aacf531b2..5712a924ae 100644 --- a/doc/usage/index.rst +++ b/doc/usage/index.rst @@ -45,6 +45,7 @@ Shell commands pstore qfw reset + rng sbi sf scp03 diff --git a/doc/usage/rng.rst b/doc/usage/rng.rst new file mode 100644 index 0000000000..87758f7d66 --- /dev/null +++ b/doc/usage/rng.rst @@ -0,0 +1,25 @@ +.. SPDX-License-Identifier: GPL-2.0+ + +rng command +=========== + +Synopsis +-------- + +:: + + rng [devnum [n]] + +Description +----------- + +The *rng* command reads the random number generator(RNG) device and +prints the random bytes read on the console. + +devnum + The RNG device from which the random bytes are to be + read. Defaults to 0. + +n + Number of random bytes to be read and displayed on the + console. Default value is 0x40.
The 'rng' command dumps a number of random bytes on the console. Add a set of tests for the 'rng' command. The test function performs basic sanity testing of the command.
Since a unit test is being added for the command, enable it by default in the sandbox platforms.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org Reviewed-by: Simon Glass sjg@chromium.org ---
Changes since V3: None
cmd/Kconfig | 1 + test/dm/rng.c | 29 +++++++++++++++++++++++++++++ 2 files changed, 30 insertions(+)
diff --git a/cmd/Kconfig b/cmd/Kconfig index 5e25e45fd2..47f1e23ef0 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -1810,6 +1810,7 @@ config CMD_GETTIME config CMD_RNG bool "rng command" depends on DM_RNG + default y if SANDBOX select HEXDUMP help Print bytes from the hardware random number generator. diff --git a/test/dm/rng.c b/test/dm/rng.c index 5b34c93ed6..6d1f68848d 100644 --- a/test/dm/rng.c +++ b/test/dm/rng.c @@ -25,3 +25,32 @@ static int dm_test_rng_read(struct unit_test_state *uts) return 0; } DM_TEST(dm_test_rng_read, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT); + +/* Test the rng command */ +static int dm_test_rng_cmd(struct unit_test_state *uts) +{ + struct udevice *dev; + + ut_assertok(uclass_get_device(UCLASS_RNG, 0, &dev)); + ut_assertnonnull(dev); + + ut_assertok(console_record_reset_enable()); + + run_command("rng", 0); + ut_assert_nextlinen("00000000:"); + ut_assert_nextlinen("00000010:"); + ut_assert_nextlinen("00000020:"); + ut_assert_nextlinen("00000030:"); + ut_assert_console_end(); + + run_command("rng 0 10", 0); + ut_assert_nextlinen("00000000:"); + ut_assert_console_end(); + + run_command("rng 20", 0); + ut_assert_nextlinen("No RNG device"); + ut_assert_console_end(); + + return 0; +} +DM_TEST(dm_test_rng_cmd, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT | UT_TESTF_CONSOLE_REC);
participants (2)
-
Simon Glass -
Sughosh Ganu