| Message ID | 20220704133444.1110715-3-sughosh.ganu@linaro.org |
|---|---|
| State | Superseded |
| Delegated to: | Ilias Apalodimas |
| Headers | show |
| Series | tpm: rng: Move TPM RNG functionality to driver model | expand |
Hi Sughosh, On Mon, 4 Jul 2022 at 07: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 V5: > * Use the dev_get_parent() interface for getting the TPM device when > calling the tpm_get_random() function > > drivers/rng/Kconfig | 11 +++++++++++ > drivers/rng/Makefile | 1 + > drivers/rng/tpm_rng.c | 23 +++++++++++++++++++++++ > lib/Kconfig | 1 + > lib/tpm-v1.c | 13 +++++++------ > lib/tpm-v2.c | 6 +++--- > lib/tpm_api.c | 6 +++--- > 7 files changed, 49 insertions(+), 12 deletions(-) > create mode 100644 drivers/rng/tpm_rng.c The API change should not be needed. The code that calls the TPM routines should adapt for that. Regards, Simon
hi Simon, On Tue, 5 Jul 2022 at 15:17, Simon Glass <sjg@chromium.org> wrote: > > Hi Sughosh, > > On Mon, 4 Jul 2022 at 07: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 V5: > > * Use the dev_get_parent() interface for getting the TPM device when > > calling the tpm_get_random() function > > > > drivers/rng/Kconfig | 11 +++++++++++ > > drivers/rng/Makefile | 1 + > > drivers/rng/tpm_rng.c | 23 +++++++++++++++++++++++ > > lib/Kconfig | 1 + > > lib/tpm-v1.c | 13 +++++++------ > > lib/tpm-v2.c | 6 +++--- > > lib/tpm_api.c | 6 +++--- > > 7 files changed, 49 insertions(+), 12 deletions(-) > > create mode 100644 drivers/rng/tpm_rng.c > > The API change should not be needed. The code that calls the TPM > routines should adapt for that. Are you referring to the changes made in tpm[1,2]_get_random functions? If so, those changes in the return values have been made based on review comments from Heinrich[1]. If you are referring to some other changes, please let me know. Thanks. -sughosh [1] - https://lists.denx.de/pipermail/u-boot/2022-February/476085.html > > Regards, > Simon
On Mon, Jul 04, 2022 at 07:04:39PM +0530, Sughosh Ganu 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 V5: > * Use the dev_get_parent() interface for getting the TPM device when > calling the tpm_get_random() function > > drivers/rng/Kconfig | 11 +++++++++++ > drivers/rng/Makefile | 1 + > drivers/rng/tpm_rng.c | 23 +++++++++++++++++++++++ > lib/Kconfig | 1 + > lib/tpm-v1.c | 13 +++++++------ > lib/tpm-v2.c | 6 +++--- > lib/tpm_api.c | 6 +++--- > 7 files changed, 49 insertions(+), 12 deletions(-) > create mode 100644 drivers/rng/tpm_rng.c > > diff --git a/drivers/rng/Kconfig b/drivers/rng/Kconfig > index c10f7d345b..67c65311c7 100644 > --- a/drivers/rng/Kconfig > +++ b/drivers/rng/Kconfig > @@ -58,4 +58,15 @@ config RNG_IPROC200 > depends on DM_RNG > help > Enable random number generator for RPI4. > + > +config TPM_RNG > + bool "Enable random number generator on TPM device" > + depends on TPM > + default y > + help > + The TPM device has an inbuilt random number generator > + functionality. Enable random number generator on TPM > + devices. Maybe we discussed this on a previous version, but why do we want to have this as a config option? Code size? A TPM will always be able to generate a random number. Couldn't we compile this based on an existing TPM Kconfig option? > + > + > endif > diff --git a/drivers/rng/Makefile b/drivers/rng/Makefile > index 435b3b965a..e4ca9c4149 100644 > --- a/drivers/rng/Makefile > +++ b/drivers/rng/Makefile > @@ -11,3 +11,4 @@ obj-$(CONFIG_RNG_OPTEE) += optee_rng.o > obj-$(CONFIG_RNG_STM32MP1) += stm32mp1_rng.o > obj-$(CONFIG_RNG_ROCKCHIP) += rockchip_rng.o > obj-$(CONFIG_RNG_IPROC200) += iproc_rng200.o > +obj-$(CONFIG_TPM_RNG) += tpm_rng.o > diff --git a/drivers/rng/tpm_rng.c b/drivers/rng/tpm_rng.c > new file mode 100644 > index 0000000000..1a5e9e2e4b > --- /dev/null > +++ b/drivers/rng/tpm_rng.c > @@ -0,0 +1,23 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Copyright (c) 2022, Linaro Limited > + */ > + > +#include <dm.h> > +#include <rng.h> > +#include <tpm_api.h> > + > +static int rng_tpm_random_read(struct udevice *dev, void *data, size_t count) > +{ > + return tpm_get_random(dev_get_parent(dev), data, count); > +} > + > +static const struct dm_rng_ops tpm_rng_ops = { > + .read = rng_tpm_random_read, > +}; > + > +U_BOOT_DRIVER(tpm_rng) = { > + .name = "tpm-rng", > + .id = UCLASS_RNG, > + .ops = &tpm_rng_ops, > +}; > diff --git a/lib/Kconfig b/lib/Kconfig > index acc0ac081a..17efaa4c80 100644 > --- a/lib/Kconfig > +++ b/lib/Kconfig > @@ -358,6 +358,7 @@ source lib/crypt/Kconfig > config TPM > bool "Trusted Platform Module (TPM) Support" > depends on DM > + imply DM_RNG > help > This enables support for TPMs which can be used to provide security > features for your board. The TPM can be connected via LPC or I2C > diff --git a/lib/tpm-v1.c b/lib/tpm-v1.c > index 22a769c587..f7091e5bc7 100644 > --- a/lib/tpm-v1.c > +++ b/lib/tpm-v1.c > @@ -9,12 +9,13 @@ > #include <common.h> > #include <dm.h> > #include <log.h> > -#include <asm/unaligned.h> > -#include <u-boot/sha1.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 > @@ -892,19 +893,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..abca9a14b0 100644 > --- a/lib/tpm-v2.c > +++ b/lib/tpm-v2.c > @@ -585,19 +585,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 4ac4612c81..032f383ca0 100644 > --- a/lib/tpm_api.c > +++ b/lib/tpm_api.c > @@ -269,7 +269,7 @@ u32 tpm_get_random(struct udevice *dev, void *data, u32 count) > if (tpm_is_v1(dev)) > return tpm1_get_random(dev, data, count); > else if (tpm_is_v2(dev)) > - return -ENOSYS; /* not implemented yet */ > - else > - return -ENOSYS; > + return tpm2_get_random(dev, data, count); > + > + return -ENOSYS; > } > -- > 2.25.1 > Thanks /Ilias
Hi Sughosh, On Tue, 5 Jul 2022 at 11:23, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > hi Simon, > > On Tue, 5 Jul 2022 at 15:17, Simon Glass <sjg@chromium.org> wrote: > > > > Hi Sughosh, > > > > On Mon, 4 Jul 2022 at 07: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 V5: > > > * Use the dev_get_parent() interface for getting the TPM device when > > > calling the tpm_get_random() function > > > > > > drivers/rng/Kconfig | 11 +++++++++++ > > > drivers/rng/Makefile | 1 + > > > drivers/rng/tpm_rng.c | 23 +++++++++++++++++++++++ > > > lib/Kconfig | 1 + > > > lib/tpm-v1.c | 13 +++++++------ > > > lib/tpm-v2.c | 6 +++--- > > > lib/tpm_api.c | 6 +++--- > > > 7 files changed, 49 insertions(+), 12 deletions(-) > > > create mode 100644 drivers/rng/tpm_rng.c > > > > The API change should not be needed. The code that calls the TPM > > routines should adapt for that. > > Are you referring to the changes made in tpm[1,2]_get_random > functions? If so, those changes in the return values have been made > based on review comments from Heinrich[1]. If you are referring to > some other changes, please let me know. Thanks. I think Heinrich might have the wrong end of the stick. The TPM API returns a positive error code. We should not change it. The tpm_rng module can handle any conversion. Regards, Simon > > -sughosh > > [1] - https://lists.denx.de/pipermail/u-boot/2022-February/476085.html
diff --git a/drivers/rng/Kconfig b/drivers/rng/Kconfig index c10f7d345b..67c65311c7 100644 --- a/drivers/rng/Kconfig +++ b/drivers/rng/Kconfig @@ -58,4 +58,15 @@ config RNG_IPROC200 depends on DM_RNG help Enable random number generator for RPI4. + +config TPM_RNG + bool "Enable random number generator on TPM device" + depends on TPM + default y + help + The TPM device has an inbuilt random number generator + functionality. Enable random number generator on TPM + devices. + + endif diff --git a/drivers/rng/Makefile b/drivers/rng/Makefile index 435b3b965a..e4ca9c4149 100644 --- a/drivers/rng/Makefile +++ b/drivers/rng/Makefile @@ -11,3 +11,4 @@ obj-$(CONFIG_RNG_OPTEE) += optee_rng.o obj-$(CONFIG_RNG_STM32MP1) += stm32mp1_rng.o obj-$(CONFIG_RNG_ROCKCHIP) += rockchip_rng.o obj-$(CONFIG_RNG_IPROC200) += iproc_rng200.o +obj-$(CONFIG_TPM_RNG) += tpm_rng.o diff --git a/drivers/rng/tpm_rng.c b/drivers/rng/tpm_rng.c new file mode 100644 index 0000000000..1a5e9e2e4b --- /dev/null +++ b/drivers/rng/tpm_rng.c @@ -0,0 +1,23 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (c) 2022, Linaro Limited + */ + +#include <dm.h> +#include <rng.h> +#include <tpm_api.h> + +static int rng_tpm_random_read(struct udevice *dev, void *data, size_t count) +{ + return tpm_get_random(dev_get_parent(dev), data, count); +} + +static const struct dm_rng_ops tpm_rng_ops = { + .read = rng_tpm_random_read, +}; + +U_BOOT_DRIVER(tpm_rng) = { + .name = "tpm-rng", + .id = UCLASS_RNG, + .ops = &tpm_rng_ops, +}; diff --git a/lib/Kconfig b/lib/Kconfig index acc0ac081a..17efaa4c80 100644 --- a/lib/Kconfig +++ b/lib/Kconfig @@ -358,6 +358,7 @@ source lib/crypt/Kconfig config TPM bool "Trusted Platform Module (TPM) Support" depends on DM + imply DM_RNG help This enables support for TPMs which can be used to provide security features for your board. The TPM can be connected via LPC or I2C diff --git a/lib/tpm-v1.c b/lib/tpm-v1.c index 22a769c587..f7091e5bc7 100644 --- a/lib/tpm-v1.c +++ b/lib/tpm-v1.c @@ -9,12 +9,13 @@ #include <common.h> #include <dm.h> #include <log.h> -#include <asm/unaligned.h> -#include <u-boot/sha1.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 @@ -892,19 +893,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..abca9a14b0 100644 --- a/lib/tpm-v2.c +++ b/lib/tpm-v2.c @@ -585,19 +585,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 4ac4612c81..032f383ca0 100644 --- a/lib/tpm_api.c +++ b/lib/tpm_api.c @@ -269,7 +269,7 @@ u32 tpm_get_random(struct udevice *dev, void *data, u32 count) if (tpm_is_v1(dev)) return tpm1_get_random(dev, data, count); else if (tpm_is_v2(dev)) - return -ENOSYS; /* not implemented yet */ - else - return -ENOSYS; + return tpm2_get_random(dev, data, count); + + return -ENOSYS; }
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 V5: * Use the dev_get_parent() interface for getting the TPM device when calling the tpm_get_random() function drivers/rng/Kconfig | 11 +++++++++++ drivers/rng/Makefile | 1 + drivers/rng/tpm_rng.c | 23 +++++++++++++++++++++++ lib/Kconfig | 1 + lib/tpm-v1.c | 13 +++++++------ lib/tpm-v2.c | 6 +++--- lib/tpm_api.c | 6 +++--- 7 files changed, 49 insertions(+), 12 deletions(-) create mode 100644 drivers/rng/tpm_rng.c