diff mbox series

[v6,2/7] tpm: rng: Add driver model interface for TPM RNG device

Message ID 20220704133444.1110715-3-sughosh.ganu@linaro.org
State New
Headers show
Series tpm: rng: Move TPM RNG functionality to driver model | expand

Commit Message

Sughosh Ganu July 4, 2022, 1:34 p.m. UTC 
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

Comments

Simon Glass July 5, 2022, 9:47 a.m. UTC | #1 
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
Sughosh Ganu July 5, 2022, 5:23 p.m. UTC | #2 
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
Ilias Apalodimas July 6, 2022, 1:26 p.m. UTC | #3 
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
Simon Glass July 12, 2022, 10:58 a.m. UTC | #4 
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 mbox series

Patch

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;
 }