[PATCH v3 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. Using this patch, a couple of patches[3][4] from the series are no longer needed.
[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/#... [3] - https://lore.kernel.org/u-boot/20220228120638.678137-7-sughosh.ganu@linaro.o... [4] - https://lore.kernel.org/u-boot/20220228120638.678137-6-sughosh.ganu@linaro.o...
Changes since V2:
* Export the existing tpm*_get_random functions to the driver model instead of moving them to the drivers/rng/ directory. * Enable DM_RNG when CONFIG_TPM is enabled to build the RNG uclass code * Additional patch for a document for the 'rng' command as suggested by Simon Glass * Additional patch for unit test for the 'rng' command as suggested by Simon Glass
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/tpm/tpm-uclass.c | 60 +++++++++++++++++++++++++++-- include/tpm-v1.h | 4 +- include/tpm-v2.h | 4 +- include/tpm_api.h | 6 +-- lib/Kconfig | 1 + lib/tpm-v1.c | 28 ++++++++++---- lib/tpm-v2.c | 21 +++++++--- lib/tpm_api.c | 28 +++++++++++--- test/dm/rng.c | 29 ++++++++++++++ 14 files changed, 201 insertions(+), 80 deletions(-) create mode 100644 doc/usage/rng.rst
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: Simon Glass sjg@chromium.org ---
Changes since V2: 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);
On Fri, Mar 04, 2022 at 07:04:22PM +0530, Sughosh Ganu wrote:
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: Simon Glass sjg@chromium.org
Changes since V2: 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); -- 2.25.1
Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org
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: Simon Glass sjg@chromium.org ---
Changes since V2: 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);
On Fri, Mar 04, 2022 at 07:04:23PM +0530, Sughosh Ganu wrote:
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: Simon Glass sjg@chromium.org
Changes since V2: 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); -- 2.25.1
Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org
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); 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 */
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 */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);}
+#endif /* DM_RNG */
2.25.1
Regards, Simon
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 */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);}
+#endif /* DM_RNG */
2.25.1
Regards, Simon
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
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
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 V2: * Enable DM_RNG when CONFIG_TPM is enabled to build the RNG uclass code
drivers/tpm/tpm-uclass.c | 60 +++++++++++++++++++++++++++++++++++++--- lib/Kconfig | 1 + 2 files changed, 57 insertions(+), 4 deletions(-)
diff --git a/drivers/tpm/tpm-uclass.c b/drivers/tpm/tpm-uclass.c index f67fe1019b..d1b9e0a757 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,58 @@ int tpm_xfer(struct udevice *dev, const uint8_t *sendbuf, size_t send_size, return 0; }
+#if IS_ENABLED(CONFIG_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; + + 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; +} + +static int tpm_uclass_child_pre_probe(struct udevice *dev) +{ + int ret; + + ret = tpm_open(dev->parent); + if (ret == -EBUSY) { + log_info("TPM device already opened\n"); + } else if (ret) { + log_err("Unable to open TPM device\n"); + return ret; + } + + ret = tpm_startup(dev->parent, TPM_ST_CLEAR); + if (ret) + log_err("Unable to start TPM device\n"); + + return ret; +} +#endif /* CONFIG_TPM */ + 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_TPM) + .post_probe = tpm_uclass_post_probe, + .child_pre_probe = tpm_uclass_child_pre_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,
On Fri, 4 Mar 2022 at 06:35, 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 V2:
- Enable DM_RNG when CONFIG_TPM is enabled to build the RNG uclass code
drivers/tpm/tpm-uclass.c | 60 +++++++++++++++++++++++++++++++++++++--- lib/Kconfig | 1 + 2 files changed, 57 insertions(+), 4 deletions(-)
No new comments from last time, still needs to be addressed.
diff --git a/drivers/tpm/tpm-uclass.c b/drivers/tpm/tpm-uclass.c index f67fe1019b..d1b9e0a757 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,58 @@ int tpm_xfer(struct udevice *dev, const uint8_t *sendbuf, size_t send_size, return 0; }
+#if IS_ENABLED(CONFIG_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;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;+}
+static int tpm_uclass_child_pre_probe(struct udevice *dev) +{
int ret;ret = tpm_open(dev->parent);if (ret == -EBUSY) {log_info("TPM device already opened\n");} else if (ret) {log_err("Unable to open TPM device\n");return ret;}ret = tpm_startup(dev->parent, TPM_ST_CLEAR);if (ret)log_err("Unable to start TPM device\n");return ret;+} +#endif /* CONFIG_TPM */
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_TPM)
.post_probe = tpm_uclass_post_probe,.child_pre_probe = tpm_uclass_child_pre_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-- 2.25.1
Regards, Simon
hi Simon,
On Wed, 9 Mar 2022 at 08:05, Simon Glass sjg@chromium.org wrote:
Hi,
On Fri, 4 Mar 2022 at 06:35, 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 V2:
- Enable DM_RNG when CONFIG_TPM is enabled to build the RNG uclass code
drivers/tpm/tpm-uclass.c | 60 +++++++++++++++++++++++++++++++++++++--- lib/Kconfig | 1 + 2 files changed, 57 insertions(+), 4 deletions(-)
No new comments from last time, still needs to be addressed.
Like I mentioned in the discussion on this patch, I will remove the child_pre_probe callback, which was starting the TPM device. I will keep the addition of the RNG child device only for the u-boot proper stage, using the CONFIG_SPL_BUILD and CONFIG_TPL_BUILD guards.
-sughosh
diff --git a/drivers/tpm/tpm-uclass.c b/drivers/tpm/tpm-uclass.c index f67fe1019b..d1b9e0a757 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,58 @@ int tpm_xfer(struct udevice *dev, const uint8_t *sendbuf, size_t send_size, return 0; }
+#if IS_ENABLED(CONFIG_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;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;+}
+static int tpm_uclass_child_pre_probe(struct udevice *dev) +{
int ret;ret = tpm_open(dev->parent);if (ret == -EBUSY) {log_info("TPM device already opened\n");} else if (ret) {log_err("Unable to open TPM device\n");return ret;}ret = tpm_startup(dev->parent, TPM_ST_CLEAR);if (ret)log_err("Unable to start TPM device\n");return ret;+} +#endif /* CONFIG_TPM */
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_TPM)
.post_probe = tpm_uclass_post_probe,.child_pre_probe = tpm_uclass_child_pre_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-- 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 ---
Changes since V2: 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
On Fri, 4 Mar 2022 at 06:35, Sughosh Ganu sughosh.ganu@linaro.org wrote:
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
Changes since V2: None
board/emulation/qemu-arm/qemu-arm.c | 42 ----------------------------- 1 file changed, 42 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
On Wed, 9 Mar 2022 at 04:35, Simon Glass sjg@chromium.org wrote:
On Fri, 4 Mar 2022 at 06:35, Sughosh Ganu sughosh.ganu@linaro.org wrote:
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
Changes since V2: None
board/emulation/qemu-arm/qemu-arm.c | 42 ----------------------------- 1 file changed, 42 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org
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 ---
Changes since V2: 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 );
Hi Sughosh,
On Fri, 4 Mar 2022 at 06:35, Sughosh Ganu sughosh.ganu@linaro.org wrote:
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
Changes since V2: None
cmd/rng.c | 31 +++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-)
Please add docs to doc/ and a test for the command.
Regards, Simon
hi Simon,
On Wed, 9 Mar 2022 at 08:05, Simon Glass sjg@chromium.org wrote:
Hi Sughosh,
On Fri, 4 Mar 2022 at 06:35, Sughosh Ganu sughosh.ganu@linaro.org wrote:
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
Changes since V2: None
cmd/rng.c | 31 +++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-)
Please add docs to doc/ and a test for the command.
That is being done in patches 7 and 8 of this series.
-sughosh
Regards, Simon
On Fri, Mar 04, 2022 at 07:04:27PM +0530, Sughosh Ganu wrote:
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
Changes since V2: 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
);
2.25.1
Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org
Hi Sugosh,
On Fri, 4 Mar 2022 at 06:35, Sughosh Ganu sughosh.ganu@linaro.org wrote:
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
Changes since V2: 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) {
Devices are numbered by aliases, so you should use uclass_get_device_by_seq() here.
printf("No RNG device\n"); return CMD_RET_FAILURE; }
if (argc >= 2)n = hextoul(argv[1], NULL);buf = malloc(n);
No need to malloc(), just set a limit for (say 64) bytes. See how cmd_mem.c does it.
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);
2.25.1
Regards, Simon
hi Simon,
On Wed, 9 Mar 2022 at 21:02, Simon Glass sjg@chromium.org wrote:
Hi Sugosh,
On Fri, 4 Mar 2022 at 06:35, Sughosh Ganu sughosh.ganu@linaro.org wrote:
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
Changes since V2: 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) {Devices are numbered by aliases, so you should use uclass_get_device_by_seq() here.
printf("No RNG device\n"); return CMD_RET_FAILURE; }
if (argc >= 2)n = hextoul(argv[1], NULL);buf = malloc(n);No need to malloc(), just set a limit for (say 64) bytes. See how cmd_mem.c does it.
These changes were not made as part of this patch. This is already existing code. I will make the changes that you suggest nonetheless. Btw, can you please take a look at the v4 patchset for this series. Thanks.
-sughosh
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);
2.25.1
Regards, Simon
Hi Sughosh,
On Thu, 10 Mar 2022 at 05:43, Sughosh Ganu sughosh.ganu@linaro.org wrote:
hi Simon,
On Wed, 9 Mar 2022 at 21:02, Simon Glass sjg@chromium.org wrote:
Hi Sugosh,
On Fri, 4 Mar 2022 at 06:35, Sughosh Ganu sughosh.ganu@linaro.org wrote:
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
Changes since V2: 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) {Devices are numbered by aliases, so you should use uclass_get_device_by_seq() here.
printf("No RNG device\n"); return CMD_RET_FAILURE; }
if (argc >= 2)n = hextoul(argv[1], NULL);buf = malloc(n);No need to malloc(), just set a limit for (say 64) bytes. See how cmd_mem.c does it.
These changes were not made as part of this patch. This is already existing code. I will make the changes that you suggest nonetheless. Btw, can you please take a look at the v4 patchset for this series. Thanks.
Ah I see, then you could do it as another patch.
Regards, Simon
Add a usage document for the 'rng' u-boot command.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org ---
Changes since V2: 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.
On Fri, 4 Mar 2022 at 06:35, Sughosh Ganu sughosh.ganu@linaro.org wrote:
Add a usage document for the 'rng' u-boot command.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
Changes since V2: None
doc/usage/index.rst | 1 + doc/usage/rng.rst | 25 +++++++++++++++++++++++++ 2 files changed, 26 insertions(+) create mode 100644 doc/usage/rng.rst
Reviewed-by: Simon Glass sjg@chromium.org
On Fri, Mar 04, 2022 at 07:04:28PM +0530, Sughosh Ganu wrote:
Add a usage document for the 'rng' u-boot command.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
Changes since V2: 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.
-- 2.25.1
Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org
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 ---
Changes since V2: 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);
Hi Sughosh,
On Fri, 4 Mar 2022 at 06:35, Sughosh Ganu sughosh.ganu@linaro.org wrote:
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
Changes since V2: None
cmd/Kconfig | 1 + test/dm/rng.c | 29 +++++++++++++++++++++++++++++ 2 files changed, 30 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
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:");
This is good enough for testing, except that here you don't have any actual data. How come?
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);
2.25.1
Regards, SImon
hi Simon,
On Wed, 9 Mar 2022 at 08:06, Simon Glass sjg@chromium.org wrote:
Hi Sughosh,
On Fri, 4 Mar 2022 at 06:35, Sughosh Ganu sughosh.ganu@linaro.org wrote:
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
Changes since V2: None
cmd/Kconfig | 1 + test/dm/rng.c | 29 +++++++++++++++++++++++++++++ 2 files changed, 30 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
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:");This is good enough for testing, except that here you don't have any actual data. How come?
This being a test for a random number device, we cannot anticipate what data we will be getting. The sandbox RNG driver does not return a fixed set of values on each invocation of the command.
-sughosh
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);
2.25.1
Regards, SImon
Hi Sughosh,
On Tue, 8 Mar 2022 at 23:18, Sughosh Ganu sughosh.ganu@linaro.org wrote:
hi Simon,
On Wed, 9 Mar 2022 at 08:06, Simon Glass sjg@chromium.org wrote:
Hi Sughosh,
On Fri, 4 Mar 2022 at 06:35, Sughosh Ganu sughosh.ganu@linaro.org wrote:
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
Changes since V2: None
cmd/Kconfig | 1 + test/dm/rng.c | 29 +++++++++++++++++++++++++++++ 2 files changed, 30 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
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:");This is good enough for testing, except that here you don't have any actual data. How come?
This being a test for a random number device, we cannot anticipate what data we will be getting. The sandbox RNG driver does not return a fixed set of values on each invocation of the command.
How come? Can you fix that by setting the random seed at the start of the test?
Regards, Simon
-sughosh
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);
2.25.1
Regards, SImon
hi Simon,
On Sat, 12 Mar 2022 at 08:14, Simon Glass sjg@chromium.org wrote:
Hi Sughosh,
On Tue, 8 Mar 2022 at 23:18, Sughosh Ganu sughosh.ganu@linaro.org wrote:
hi Simon,
On Wed, 9 Mar 2022 at 08:06, Simon Glass sjg@chromium.org wrote:
Hi Sughosh,
On Fri, 4 Mar 2022 at 06:35, Sughosh Ganu sughosh.ganu@linaro.org wrote:
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
Changes since V2: None
cmd/Kconfig | 1 + test/dm/rng.c | 29 +++++++++++++++++++++++++++++ 2 files changed, 30 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
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:");This is good enough for testing, except that here you don't have any actual data. How come?
This being a test for a random number device, we cannot anticipate what data we will be getting. The sandbox RNG driver does not return a fixed set of values on each invocation of the command.
How come? Can you fix that by setting the random seed at the start of the test?
Well I can, but do we really need to do that. I think the current test that we have for the rng command and the uclass is fine.
-sughosh
Regards, Simon
-sughosh
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);
2.25.1
Regards, SImon
Hi Sughosh,
On Sun, 13 Mar 2022 at 05:08, Sughosh Ganu sughosh.ganu@linaro.org wrote:
hi Simon,
On Sat, 12 Mar 2022 at 08:14, Simon Glass sjg@chromium.org wrote:
Hi Sughosh,
On Tue, 8 Mar 2022 at 23:18, Sughosh Ganu sughosh.ganu@linaro.org wrote:
hi Simon,
On Wed, 9 Mar 2022 at 08:06, Simon Glass sjg@chromium.org wrote:
Hi Sughosh,
On Fri, 4 Mar 2022 at 06:35, Sughosh Ganu sughosh.ganu@linaro.org wrote:
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
Changes since V2: None
cmd/Kconfig | 1 + test/dm/rng.c | 29 +++++++++++++++++++++++++++++ 2 files changed, 30 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
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:");This is good enough for testing, except that here you don't have any actual data. How come?
This being a test for a random number device, we cannot anticipate what data we will be getting. The sandbox RNG driver does not return a fixed set of values on each invocation of the command.
How come? Can you fix that by setting the random seed at the start of the test?
Well I can, but do we really need to do that. I think the current test that we have for the rng command and the uclass is fine.
It is OK, but not great, since if there is a bug in the data output or in the collection of the random data, then you will not find it with this test.
Regards, Simon
hi Heinrich,
On Fri, 4 Mar 2022 at 19:05, Sughosh Ganu sughosh.ganu@linaro.org wrote:
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
Changes since V2: None
cmd/Kconfig | 1 + test/dm/rng.c | 29 +++++++++++++++++++++++++++++ 2 files changed, 30 insertions(+)
Can you please pick this up, as this is unrelated to the TPM RNG changes which are still under discussion. Thanks.
-sughosh
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);
2.25.1
participants (3)
-
Ilias Apalodimas -
Simon Glass -
Sughosh Ganu