Re: [PATCH v2 07/10] tpm: rng: Move the TPM RNG functionality to driver model

2022-03-04 Thread Sughosh Ganu
hi Simon,

On Fri, 4 Mar 2022 at 08:08, Simon Glass  wrote:
>
> Hi Sughosh,
>
> On Thu, 3 Mar 2022 at 05:07, Sughosh Ganu  wrote:
> >
> > hi Simon,
> >
> > On Thu, 3 Mar 2022 at 09:17, Simon Glass  wrote:
> > >
> > > Hi Sughosh,
> > >
> > > On Tue, 1 Mar 2022 at 21:36, Sughosh Ganu  wrote:
> > > >
> > > > hi Simon,
> > > >
> > > > On Tue, 1 Mar 2022 at 20:28, Simon Glass  wrote:
> > > > >
> > > > > Hi Sughosh,
> > > > >
> > > > > On Mon, 28 Feb 2022 at 05:07, Sughosh Ganu  
> > > > > wrote:
> > > > > >
> > > > > > Currently, the TPM random number generator(RNG) functions are 
> > > > > > defined
> > > > > > as part of the library functions under the corresponding tpm files 
> > > > > > for
> > > > > > tpmv1 and tpmv2. Move the RNG functionality under TPM RNG drivers
> > > > > > complying with the driver model.
> > > > > >
> > > > > > Also make changes to the tpm_get_random function to have it call the
> > > > > > TPM RNG driver functions instead of the library functions.
> > > > > >
> > > > > > Signed-off-by: Sughosh Ganu 
> > > > > > ---
> > > > > >
> > > > > > Changes since V1:
> > > > > > * Added existing copyrights for the rng functions taken from the tpm
> > > > > >   library routines
> > > > > > * Return -EIO for TPM command returning an error
> > > > > > * Simplify the logic in tpm_get_random based on the review comments
> > > > > >   from Ilias
> > > > > >
> > > > > >  drivers/rng/Makefile   |  1 +
> > > > > >  drivers/rng/tpm1_rng.c | 87 
> > > > > > ++
> > > > > >  drivers/rng/tpm2_rng.c | 86 
> > > > > > +
> > > > > >  lib/tpm-v1.c   | 44 -
> > > > > >  lib/tpm-v2.c   | 44 -
> > > > > >  lib/tpm_api.c  | 23 +--
> > > > > >  6 files changed, 193 insertions(+), 92 deletions(-)
> > > > > >  create mode 100644 drivers/rng/tpm1_rng.c
> > > > > >  create mode 100644 drivers/rng/tpm2_rng.c
> > > > > >
> > > > > > diff --git a/drivers/rng/Makefile b/drivers/rng/Makefile
> > > > > > index 39f7ee3f03..129cfbd006 100644
> > > > > > --- a/drivers/rng/Makefile
> > > > > > +++ b/drivers/rng/Makefile
> > > > > > @@ -10,3 +10,4 @@ obj-$(CONFIG_RNG_MSM) += msm_rng.o
> > > > > >  obj-$(CONFIG_RNG_STM32MP1) += stm32mp1_rng.o
> > > > > >  obj-$(CONFIG_RNG_ROCKCHIP) += rockchip_rng.o
> > > > > >  obj-$(CONFIG_RNG_IPROC200) += iproc_rng200.o
> > > > > > +obj-$(CONFIG_TPM) += tpm1_rng.o tpm2_rng.o
> > > > > > diff --git a/drivers/rng/tpm1_rng.c b/drivers/rng/tpm1_rng.c
> > > > > > new file mode 100644
> > > > > > index 00..7e629756b3
> > > > > > --- /dev/null
> > > > > > +++ b/drivers/rng/tpm1_rng.c
> > > > > > @@ -0,0 +1,87 @@
> > > > > > +// SPDX-License-Identifier: GPL-2.0+
> > > > > > +/*
> > > > > > + * Copyright (c) 2013 The Chromium OS Authors.
> > > > > > + * Coypright (c) 2013 Guntermann & Drunck GmbH
> > > > > > + * Copyright (c) 2022, Linaro Limited
> > > > > > + */
> > > > > > +
> > > > > > +#include 
> > > > > > +#include 
> > > > > > +#include 
> > > > > > +#include 
> > > > > > +#include 
> > > > > > +
> > > > > > +#include 
> > > > > > +
> > > > > > +#define TPM_HEADER_SIZE10
> > > > > > +
> > > > > > +#define TPMV1_DATA_OFFSET  14
> > > > > > +
> > > > > > +/**
> > > > > > + * tpm1_rng_read() - Read the random bytes from TPMv1 device
> > > > > > + * @param dev  TPMv1 RNG device
> > > > > > + * @param data data buffer to write random bytes
> > > > > > + * @param countnumber of random bytes to read from
> > > > > > + *  the device
> > > > > > + *
> > > > > > + * Function to read the random bytes from the RNG pseudo device
> > > > > > + * built into the TPMv1 device. Reads 'count' number of bytes
> > > > > > + * from the random number generator and copies them into the
> > > > > > + * 'data' buffer.
> > > > > > + *
> > > > > > + * Return: 0 if OK, -ve on error.
> > > > > > + *
> > > > > > + */
> > > > > > +static int tpm1_rng_read(struct udevice *dev, void *data, size_t 
> > > > > > count)
> > > > > > +{
> > > > > > +   const u8 command[14] = {
> > > > > > +   0x0, 0xc1,  /* TPM_TAG */
> > > > > > +   0x0, 0x0, 0x0, 0xe, /* parameter size */
> > > > > > +   0x0, 0x0, 0x0, 0x46,/* TPM_COMMAND_CODE */
> > > > > > +   };
> > > > > > +   const size_t length_offset = TPM_HEADER_SIZE;
> > > > > > +   const size_t data_size_offset = TPM_HEADER_SIZE;
> > > > > > +   const size_t data_offset = TPMV1_DATA_OFFSET;
> > > > > > +   u8 buf[COMMAND_BUFFER_SIZE], response[COMMAND_BUFFER_SIZE];
> > > > > > +   size_t response_length = sizeof(response);
> > > > > > +   u32 data_size;
> > > > > > +   u8 *out = data;
> > > > > > +
> > > > >
> > > > > The current model is that all TPM calls are set up in lib/tpm and I
> > > > > don't think we should change it. You should be able to move these
> > > > > 

Re: [PATCH v2 07/10] tpm: rng: Move the TPM RNG functionality to driver model

2022-03-03 Thread Simon Glass
Hi Sughosh,

On Thu, 3 Mar 2022 at 05:07, Sughosh Ganu  wrote:
>
> hi Simon,
>
> On Thu, 3 Mar 2022 at 09:17, Simon Glass  wrote:
> >
> > Hi Sughosh,
> >
> > On Tue, 1 Mar 2022 at 21:36, Sughosh Ganu  wrote:
> > >
> > > hi Simon,
> > >
> > > On Tue, 1 Mar 2022 at 20:28, Simon Glass  wrote:
> > > >
> > > > Hi Sughosh,
> > > >
> > > > On Mon, 28 Feb 2022 at 05:07, Sughosh Ganu  
> > > > wrote:
> > > > >
> > > > > Currently, the TPM random number generator(RNG) functions are defined
> > > > > as part of the library functions under the corresponding tpm files for
> > > > > tpmv1 and tpmv2. Move the RNG functionality under TPM RNG drivers
> > > > > complying with the driver model.
> > > > >
> > > > > Also make changes to the tpm_get_random function to have it call the
> > > > > TPM RNG driver functions instead of the library functions.
> > > > >
> > > > > Signed-off-by: Sughosh Ganu 
> > > > > ---
> > > > >
> > > > > Changes since V1:
> > > > > * Added existing copyrights for the rng functions taken from the tpm
> > > > >   library routines
> > > > > * Return -EIO for TPM command returning an error
> > > > > * Simplify the logic in tpm_get_random based on the review comments
> > > > >   from Ilias
> > > > >
> > > > >  drivers/rng/Makefile   |  1 +
> > > > >  drivers/rng/tpm1_rng.c | 87 
> > > > > ++
> > > > >  drivers/rng/tpm2_rng.c | 86 +
> > > > >  lib/tpm-v1.c   | 44 -
> > > > >  lib/tpm-v2.c   | 44 -
> > > > >  lib/tpm_api.c  | 23 +--
> > > > >  6 files changed, 193 insertions(+), 92 deletions(-)
> > > > >  create mode 100644 drivers/rng/tpm1_rng.c
> > > > >  create mode 100644 drivers/rng/tpm2_rng.c
> > > > >
> > > > > diff --git a/drivers/rng/Makefile b/drivers/rng/Makefile
> > > > > index 39f7ee3f03..129cfbd006 100644
> > > > > --- a/drivers/rng/Makefile
> > > > > +++ b/drivers/rng/Makefile
> > > > > @@ -10,3 +10,4 @@ obj-$(CONFIG_RNG_MSM) += msm_rng.o
> > > > >  obj-$(CONFIG_RNG_STM32MP1) += stm32mp1_rng.o
> > > > >  obj-$(CONFIG_RNG_ROCKCHIP) += rockchip_rng.o
> > > > >  obj-$(CONFIG_RNG_IPROC200) += iproc_rng200.o
> > > > > +obj-$(CONFIG_TPM) += tpm1_rng.o tpm2_rng.o
> > > > > diff --git a/drivers/rng/tpm1_rng.c b/drivers/rng/tpm1_rng.c
> > > > > new file mode 100644
> > > > > index 00..7e629756b3
> > > > > --- /dev/null
> > > > > +++ b/drivers/rng/tpm1_rng.c
> > > > > @@ -0,0 +1,87 @@
> > > > > +// SPDX-License-Identifier: GPL-2.0+
> > > > > +/*
> > > > > + * Copyright (c) 2013 The Chromium OS Authors.
> > > > > + * Coypright (c) 2013 Guntermann & Drunck GmbH
> > > > > + * Copyright (c) 2022, Linaro Limited
> > > > > + */
> > > > > +
> > > > > +#include 
> > > > > +#include 
> > > > > +#include 
> > > > > +#include 
> > > > > +#include 
> > > > > +
> > > > > +#include 
> > > > > +
> > > > > +#define TPM_HEADER_SIZE10
> > > > > +
> > > > > +#define TPMV1_DATA_OFFSET  14
> > > > > +
> > > > > +/**
> > > > > + * tpm1_rng_read() - Read the random bytes from TPMv1 device
> > > > > + * @param dev  TPMv1 RNG device
> > > > > + * @param data data buffer to write random bytes
> > > > > + * @param countnumber of random bytes to read from
> > > > > + *  the device
> > > > > + *
> > > > > + * Function to read the random bytes from the RNG pseudo device
> > > > > + * built into the TPMv1 device. Reads 'count' number of bytes
> > > > > + * from the random number generator and copies them into the
> > > > > + * 'data' buffer.
> > > > > + *
> > > > > + * Return: 0 if OK, -ve on error.
> > > > > + *
> > > > > + */
> > > > > +static int tpm1_rng_read(struct udevice *dev, void *data, size_t 
> > > > > count)
> > > > > +{
> > > > > +   const u8 command[14] = {
> > > > > +   0x0, 0xc1,  /* TPM_TAG */
> > > > > +   0x0, 0x0, 0x0, 0xe, /* parameter size */
> > > > > +   0x0, 0x0, 0x0, 0x46,/* TPM_COMMAND_CODE */
> > > > > +   };
> > > > > +   const size_t length_offset = TPM_HEADER_SIZE;
> > > > > +   const size_t data_size_offset = TPM_HEADER_SIZE;
> > > > > +   const size_t data_offset = TPMV1_DATA_OFFSET;
> > > > > +   u8 buf[COMMAND_BUFFER_SIZE], response[COMMAND_BUFFER_SIZE];
> > > > > +   size_t response_length = sizeof(response);
> > > > > +   u32 data_size;
> > > > > +   u8 *out = data;
> > > > > +
> > > >
> > > > The current model is that all TPM calls are set up in lib/tpm and I
> > > > don't think we should change it. You should be able to move these
> > > > functions into lib/tpm and add your random_read function to tpm_api.h
> > >
> > > I moved these functions under separate drivers as I thought that
> > > looked cleaner as against exporting the driver interface in
> >
> > But you are now creating TPM messages in a different file so I don't
> > think it is cleaner. The message pack/unpack 

Re: [PATCH v2 07/10] tpm: rng: Move the TPM RNG functionality to driver model

2022-03-03 Thread Sughosh Ganu
hi Simon,

On Thu, 3 Mar 2022 at 09:17, Simon Glass  wrote:
>
> Hi Sughosh,
>
> On Tue, 1 Mar 2022 at 21:36, Sughosh Ganu  wrote:
> >
> > hi Simon,
> >
> > On Tue, 1 Mar 2022 at 20:28, Simon Glass  wrote:
> > >
> > > Hi Sughosh,
> > >
> > > On Mon, 28 Feb 2022 at 05:07, Sughosh Ganu  
> > > wrote:
> > > >
> > > > Currently, the TPM random number generator(RNG) functions are defined
> > > > as part of the library functions under the corresponding tpm files for
> > > > tpmv1 and tpmv2. Move the RNG functionality under TPM RNG drivers
> > > > complying with the driver model.
> > > >
> > > > Also make changes to the tpm_get_random function to have it call the
> > > > TPM RNG driver functions instead of the library functions.
> > > >
> > > > Signed-off-by: Sughosh Ganu 
> > > > ---
> > > >
> > > > Changes since V1:
> > > > * Added existing copyrights for the rng functions taken from the tpm
> > > >   library routines
> > > > * Return -EIO for TPM command returning an error
> > > > * Simplify the logic in tpm_get_random based on the review comments
> > > >   from Ilias
> > > >
> > > >  drivers/rng/Makefile   |  1 +
> > > >  drivers/rng/tpm1_rng.c | 87 ++
> > > >  drivers/rng/tpm2_rng.c | 86 +
> > > >  lib/tpm-v1.c   | 44 -
> > > >  lib/tpm-v2.c   | 44 -
> > > >  lib/tpm_api.c  | 23 +--
> > > >  6 files changed, 193 insertions(+), 92 deletions(-)
> > > >  create mode 100644 drivers/rng/tpm1_rng.c
> > > >  create mode 100644 drivers/rng/tpm2_rng.c
> > > >
> > > > diff --git a/drivers/rng/Makefile b/drivers/rng/Makefile
> > > > index 39f7ee3f03..129cfbd006 100644
> > > > --- a/drivers/rng/Makefile
> > > > +++ b/drivers/rng/Makefile
> > > > @@ -10,3 +10,4 @@ obj-$(CONFIG_RNG_MSM) += msm_rng.o
> > > >  obj-$(CONFIG_RNG_STM32MP1) += stm32mp1_rng.o
> > > >  obj-$(CONFIG_RNG_ROCKCHIP) += rockchip_rng.o
> > > >  obj-$(CONFIG_RNG_IPROC200) += iproc_rng200.o
> > > > +obj-$(CONFIG_TPM) += tpm1_rng.o tpm2_rng.o
> > > > diff --git a/drivers/rng/tpm1_rng.c b/drivers/rng/tpm1_rng.c
> > > > new file mode 100644
> > > > index 00..7e629756b3
> > > > --- /dev/null
> > > > +++ b/drivers/rng/tpm1_rng.c
> > > > @@ -0,0 +1,87 @@
> > > > +// SPDX-License-Identifier: GPL-2.0+
> > > > +/*
> > > > + * Copyright (c) 2013 The Chromium OS Authors.
> > > > + * Coypright (c) 2013 Guntermann & Drunck GmbH
> > > > + * Copyright (c) 2022, Linaro Limited
> > > > + */
> > > > +
> > > > +#include 
> > > > +#include 
> > > > +#include 
> > > > +#include 
> > > > +#include 
> > > > +
> > > > +#include 
> > > > +
> > > > +#define TPM_HEADER_SIZE10
> > > > +
> > > > +#define TPMV1_DATA_OFFSET  14
> > > > +
> > > > +/**
> > > > + * tpm1_rng_read() - Read the random bytes from TPMv1 device
> > > > + * @param dev  TPMv1 RNG device
> > > > + * @param data data buffer to write random bytes
> > > > + * @param countnumber of random bytes to read from
> > > > + *  the device
> > > > + *
> > > > + * Function to read the random bytes from the RNG pseudo device
> > > > + * built into the TPMv1 device. Reads 'count' number of bytes
> > > > + * from the random number generator and copies them into the
> > > > + * 'data' buffer.
> > > > + *
> > > > + * Return: 0 if OK, -ve on error.
> > > > + *
> > > > + */
> > > > +static int tpm1_rng_read(struct udevice *dev, void *data, size_t count)
> > > > +{
> > > > +   const u8 command[14] = {
> > > > +   0x0, 0xc1,  /* TPM_TAG */
> > > > +   0x0, 0x0, 0x0, 0xe, /* parameter size */
> > > > +   0x0, 0x0, 0x0, 0x46,/* TPM_COMMAND_CODE */
> > > > +   };
> > > > +   const size_t length_offset = TPM_HEADER_SIZE;
> > > > +   const size_t data_size_offset = TPM_HEADER_SIZE;
> > > > +   const size_t data_offset = TPMV1_DATA_OFFSET;
> > > > +   u8 buf[COMMAND_BUFFER_SIZE], response[COMMAND_BUFFER_SIZE];
> > > > +   size_t response_length = sizeof(response);
> > > > +   u32 data_size;
> > > > +   u8 *out = data;
> > > > +
> > >
> > > The current model is that all TPM calls are set up in lib/tpm and I
> > > don't think we should change it. You should be able to move these
> > > functions into lib/tpm and add your random_read function to tpm_api.h
> >
> > I moved these functions under separate drivers as I thought that
> > looked cleaner as against exporting the driver interface in
>
> But you are now creating TPM messages in a different file so I don't
> think it is cleaner. The message pack/unpack should happen in the
> tpm_... functions.
>
> > tpm-v{1,2}.c. If you strongly feel that these should remain under
> > lib/tpm, I will make the change. But I believe you do not have a
> > problem with exporting these rng functions as part of the driver
> > model. We do need that so that the EFI_RNG_PROTOCOL can use these for

Re: [PATCH v2 07/10] tpm: rng: Move the TPM RNG functionality to driver model

2022-03-02 Thread Simon Glass
Hi Sughosh,

On Tue, 1 Mar 2022 at 21:36, Sughosh Ganu  wrote:
>
> hi Simon,
>
> On Tue, 1 Mar 2022 at 20:28, Simon Glass  wrote:
> >
> > Hi Sughosh,
> >
> > On Mon, 28 Feb 2022 at 05:07, Sughosh Ganu  wrote:
> > >
> > > Currently, the TPM random number generator(RNG) functions are defined
> > > as part of the library functions under the corresponding tpm files for
> > > tpmv1 and tpmv2. Move the RNG functionality under TPM RNG drivers
> > > complying with the driver model.
> > >
> > > Also make changes to the tpm_get_random function to have it call the
> > > TPM RNG driver functions instead of the library functions.
> > >
> > > Signed-off-by: Sughosh Ganu 
> > > ---
> > >
> > > Changes since V1:
> > > * Added existing copyrights for the rng functions taken from the tpm
> > >   library routines
> > > * Return -EIO for TPM command returning an error
> > > * Simplify the logic in tpm_get_random based on the review comments
> > >   from Ilias
> > >
> > >  drivers/rng/Makefile   |  1 +
> > >  drivers/rng/tpm1_rng.c | 87 ++
> > >  drivers/rng/tpm2_rng.c | 86 +
> > >  lib/tpm-v1.c   | 44 -
> > >  lib/tpm-v2.c   | 44 -
> > >  lib/tpm_api.c  | 23 +--
> > >  6 files changed, 193 insertions(+), 92 deletions(-)
> > >  create mode 100644 drivers/rng/tpm1_rng.c
> > >  create mode 100644 drivers/rng/tpm2_rng.c
> > >
> > > diff --git a/drivers/rng/Makefile b/drivers/rng/Makefile
> > > index 39f7ee3f03..129cfbd006 100644
> > > --- a/drivers/rng/Makefile
> > > +++ b/drivers/rng/Makefile
> > > @@ -10,3 +10,4 @@ obj-$(CONFIG_RNG_MSM) += msm_rng.o
> > >  obj-$(CONFIG_RNG_STM32MP1) += stm32mp1_rng.o
> > >  obj-$(CONFIG_RNG_ROCKCHIP) += rockchip_rng.o
> > >  obj-$(CONFIG_RNG_IPROC200) += iproc_rng200.o
> > > +obj-$(CONFIG_TPM) += tpm1_rng.o tpm2_rng.o
> > > diff --git a/drivers/rng/tpm1_rng.c b/drivers/rng/tpm1_rng.c
> > > new file mode 100644
> > > index 00..7e629756b3
> > > --- /dev/null
> > > +++ b/drivers/rng/tpm1_rng.c
> > > @@ -0,0 +1,87 @@
> > > +// SPDX-License-Identifier: GPL-2.0+
> > > +/*
> > > + * Copyright (c) 2013 The Chromium OS Authors.
> > > + * Coypright (c) 2013 Guntermann & Drunck GmbH
> > > + * Copyright (c) 2022, Linaro Limited
> > > + */
> > > +
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +
> > > +#include 
> > > +
> > > +#define TPM_HEADER_SIZE10
> > > +
> > > +#define TPMV1_DATA_OFFSET  14
> > > +
> > > +/**
> > > + * tpm1_rng_read() - Read the random bytes from TPMv1 device
> > > + * @param dev  TPMv1 RNG device
> > > + * @param data data buffer to write random bytes
> > > + * @param countnumber of random bytes to read from
> > > + *  the device
> > > + *
> > > + * Function to read the random bytes from the RNG pseudo device
> > > + * built into the TPMv1 device. Reads 'count' number of bytes
> > > + * from the random number generator and copies them into the
> > > + * 'data' buffer.
> > > + *
> > > + * Return: 0 if OK, -ve on error.
> > > + *
> > > + */
> > > +static int tpm1_rng_read(struct udevice *dev, void *data, size_t count)
> > > +{
> > > +   const u8 command[14] = {
> > > +   0x0, 0xc1,  /* TPM_TAG */
> > > +   0x0, 0x0, 0x0, 0xe, /* parameter size */
> > > +   0x0, 0x0, 0x0, 0x46,/* TPM_COMMAND_CODE */
> > > +   };
> > > +   const size_t length_offset = TPM_HEADER_SIZE;
> > > +   const size_t data_size_offset = TPM_HEADER_SIZE;
> > > +   const size_t data_offset = TPMV1_DATA_OFFSET;
> > > +   u8 buf[COMMAND_BUFFER_SIZE], response[COMMAND_BUFFER_SIZE];
> > > +   size_t response_length = sizeof(response);
> > > +   u32 data_size;
> > > +   u8 *out = data;
> > > +
> >
> > The current model is that all TPM calls are set up in lib/tpm and I
> > don't think we should change it. You should be able to move these
> > functions into lib/tpm and add your random_read function to tpm_api.h
>
> I moved these functions under separate drivers as I thought that
> looked cleaner as against exporting the driver interface in

But you are now creating TPM messages in a different file so I don't
think it is cleaner. The message pack/unpack should happen in the
tpm_... functions.

> tpm-v{1,2}.c. If you strongly feel that these should remain under
> lib/tpm, I will make the change. But I believe you do not have a
> problem with exporting these rng functions as part of the driver
> model. We do need that so that the EFI_RNG_PROTOCOL can use these for
> getting the random bytes.

I think you might misunderstand me. I mean that the code that calls
pack_byte_string() should be in lib/ like the other code. I agree that
the driver should be were you have put it, I just don't like having
the tpm message creation in that driver. It should call a tpm_...

Re: [PATCH v2 07/10] tpm: rng: Move the TPM RNG functionality to driver model

2022-03-01 Thread Sughosh Ganu
hi Simon,

On Tue, 1 Mar 2022 at 20:28, Simon Glass  wrote:
>
> Hi Sughosh,
>
> On Mon, 28 Feb 2022 at 05:07, Sughosh Ganu  wrote:
> >
> > Currently, the TPM random number generator(RNG) functions are defined
> > as part of the library functions under the corresponding tpm files for
> > tpmv1 and tpmv2. Move the RNG functionality under TPM RNG drivers
> > complying with the driver model.
> >
> > Also make changes to the tpm_get_random function to have it call the
> > TPM RNG driver functions instead of the library functions.
> >
> > Signed-off-by: Sughosh Ganu 
> > ---
> >
> > Changes since V1:
> > * Added existing copyrights for the rng functions taken from the tpm
> >   library routines
> > * Return -EIO for TPM command returning an error
> > * Simplify the logic in tpm_get_random based on the review comments
> >   from Ilias
> >
> >  drivers/rng/Makefile   |  1 +
> >  drivers/rng/tpm1_rng.c | 87 ++
> >  drivers/rng/tpm2_rng.c | 86 +
> >  lib/tpm-v1.c   | 44 -
> >  lib/tpm-v2.c   | 44 -
> >  lib/tpm_api.c  | 23 +--
> >  6 files changed, 193 insertions(+), 92 deletions(-)
> >  create mode 100644 drivers/rng/tpm1_rng.c
> >  create mode 100644 drivers/rng/tpm2_rng.c
> >
> > diff --git a/drivers/rng/Makefile b/drivers/rng/Makefile
> > index 39f7ee3f03..129cfbd006 100644
> > --- a/drivers/rng/Makefile
> > +++ b/drivers/rng/Makefile
> > @@ -10,3 +10,4 @@ obj-$(CONFIG_RNG_MSM) += msm_rng.o
> >  obj-$(CONFIG_RNG_STM32MP1) += stm32mp1_rng.o
> >  obj-$(CONFIG_RNG_ROCKCHIP) += rockchip_rng.o
> >  obj-$(CONFIG_RNG_IPROC200) += iproc_rng200.o
> > +obj-$(CONFIG_TPM) += tpm1_rng.o tpm2_rng.o
> > diff --git a/drivers/rng/tpm1_rng.c b/drivers/rng/tpm1_rng.c
> > new file mode 100644
> > index 00..7e629756b3
> > --- /dev/null
> > +++ b/drivers/rng/tpm1_rng.c
> > @@ -0,0 +1,87 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright (c) 2013 The Chromium OS Authors.
> > + * Coypright (c) 2013 Guntermann & Drunck GmbH
> > + * Copyright (c) 2022, Linaro Limited
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include 
> > +
> > +#define TPM_HEADER_SIZE10
> > +
> > +#define TPMV1_DATA_OFFSET  14
> > +
> > +/**
> > + * tpm1_rng_read() - Read the random bytes from TPMv1 device
> > + * @param dev  TPMv1 RNG device
> > + * @param data data buffer to write random bytes
> > + * @param countnumber of random bytes to read from
> > + *  the device
> > + *
> > + * Function to read the random bytes from the RNG pseudo device
> > + * built into the TPMv1 device. Reads 'count' number of bytes
> > + * from the random number generator and copies them into the
> > + * 'data' buffer.
> > + *
> > + * Return: 0 if OK, -ve on error.
> > + *
> > + */
> > +static int tpm1_rng_read(struct udevice *dev, void *data, size_t count)
> > +{
> > +   const u8 command[14] = {
> > +   0x0, 0xc1,  /* TPM_TAG */
> > +   0x0, 0x0, 0x0, 0xe, /* parameter size */
> > +   0x0, 0x0, 0x0, 0x46,/* TPM_COMMAND_CODE */
> > +   };
> > +   const size_t length_offset = TPM_HEADER_SIZE;
> > +   const size_t data_size_offset = TPM_HEADER_SIZE;
> > +   const size_t data_offset = TPMV1_DATA_OFFSET;
> > +   u8 buf[COMMAND_BUFFER_SIZE], response[COMMAND_BUFFER_SIZE];
> > +   size_t response_length = sizeof(response);
> > +   u32 data_size;
> > +   u8 *out = data;
> > +
>
> The current model is that all TPM calls are set up in lib/tpm and I
> don't think we should change it. You should be able to move these
> functions into lib/tpm and add your random_read function to tpm_api.h

I moved these functions under separate drivers as I thought that
looked cleaner as against exporting the driver interface in
tpm-v{1,2}.c. If you strongly feel that these should remain under
lib/tpm, I will make the change. But I believe you do not have a
problem with exporting these rng functions as part of the driver
model. We do need that so that the EFI_RNG_PROTOCOL can use these for
getting the random bytes.

-sughosh

>
> Regards,
> Simon


Re: [PATCH v2 07/10] tpm: rng: Move the TPM RNG functionality to driver model

2022-03-01 Thread Simon Glass
Hi Sughosh,

On Mon, 28 Feb 2022 at 05:07, Sughosh Ganu  wrote:
>
> Currently, the TPM random number generator(RNG) functions are defined
> as part of the library functions under the corresponding tpm files for
> tpmv1 and tpmv2. Move the RNG functionality under TPM RNG drivers
> complying with the driver model.
>
> Also make changes to the tpm_get_random function to have it call the
> TPM RNG driver functions instead of the library functions.
>
> Signed-off-by: Sughosh Ganu 
> ---
>
> Changes since V1:
> * Added existing copyrights for the rng functions taken from the tpm
>   library routines
> * Return -EIO for TPM command returning an error
> * Simplify the logic in tpm_get_random based on the review comments
>   from Ilias
>
>  drivers/rng/Makefile   |  1 +
>  drivers/rng/tpm1_rng.c | 87 ++
>  drivers/rng/tpm2_rng.c | 86 +
>  lib/tpm-v1.c   | 44 -
>  lib/tpm-v2.c   | 44 -
>  lib/tpm_api.c  | 23 +--
>  6 files changed, 193 insertions(+), 92 deletions(-)
>  create mode 100644 drivers/rng/tpm1_rng.c
>  create mode 100644 drivers/rng/tpm2_rng.c
>
> diff --git a/drivers/rng/Makefile b/drivers/rng/Makefile
> index 39f7ee3f03..129cfbd006 100644
> --- a/drivers/rng/Makefile
> +++ b/drivers/rng/Makefile
> @@ -10,3 +10,4 @@ obj-$(CONFIG_RNG_MSM) += msm_rng.o
>  obj-$(CONFIG_RNG_STM32MP1) += stm32mp1_rng.o
>  obj-$(CONFIG_RNG_ROCKCHIP) += rockchip_rng.o
>  obj-$(CONFIG_RNG_IPROC200) += iproc_rng200.o
> +obj-$(CONFIG_TPM) += tpm1_rng.o tpm2_rng.o
> diff --git a/drivers/rng/tpm1_rng.c b/drivers/rng/tpm1_rng.c
> new file mode 100644
> index 00..7e629756b3
> --- /dev/null
> +++ b/drivers/rng/tpm1_rng.c
> @@ -0,0 +1,87 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) 2013 The Chromium OS Authors.
> + * Coypright (c) 2013 Guntermann & Drunck GmbH
> + * Copyright (c) 2022, Linaro Limited
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +
> +#define TPM_HEADER_SIZE10
> +
> +#define TPMV1_DATA_OFFSET  14
> +
> +/**
> + * tpm1_rng_read() - Read the random bytes from TPMv1 device
> + * @param dev  TPMv1 RNG device
> + * @param data data buffer to write random bytes
> + * @param countnumber of random bytes to read from
> + *  the device
> + *
> + * Function to read the random bytes from the RNG pseudo device
> + * built into the TPMv1 device. Reads 'count' number of bytes
> + * from the random number generator and copies them into the
> + * 'data' buffer.
> + *
> + * Return: 0 if OK, -ve on error.
> + *
> + */
> +static int tpm1_rng_read(struct udevice *dev, void *data, size_t count)
> +{
> +   const u8 command[14] = {
> +   0x0, 0xc1,  /* TPM_TAG */
> +   0x0, 0x0, 0x0, 0xe, /* parameter size */
> +   0x0, 0x0, 0x0, 0x46,/* TPM_COMMAND_CODE */
> +   };
> +   const size_t length_offset = TPM_HEADER_SIZE;
> +   const size_t data_size_offset = TPM_HEADER_SIZE;
> +   const size_t data_offset = TPMV1_DATA_OFFSET;
> +   u8 buf[COMMAND_BUFFER_SIZE], response[COMMAND_BUFFER_SIZE];
> +   size_t response_length = sizeof(response);
> +   u32 data_size;
> +   u8 *out = data;
> +

The current model is that all TPM calls are set up in lib/tpm and I
don't think we should change it. You should be able to move these
functions into lib/tpm and add your random_read function to tpm_api.h

Regards,
Simon