| Message ID | 20220304133429.1047752-7-sughosh.ganu@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | tpm: rng: Move TPM RNG functionality to driver model | expand |
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
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 );