diff mbox series

[v2,10/10] cmd: rng: Add support for selecting RNG device

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

Commit Message

Sughosh Ganu Feb. 28, 2022, 12:06 p.m. UTC
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 V1:
* Changed the help text to show order of the parameters passed
  to the rng command, based on review comment from Heinrich

 cmd/rng.c | 31 +++++++++++++++++++++++--------
 1 file changed, 23 insertions(+), 8 deletions(-)

Comments

Simon Glass March 1, 2022, 2:58 p.m. UTC | #1
Hi Sughosh,

On Mon, 28 Feb 2022 at 05:07, 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 V1:
> * Changed the help text to show order of the parameters passed
>   to the rng command, based on review comment from Heinrich
>
>  cmd/rng.c | 31 +++++++++++++++++++++++--------
>  1 file changed, 23 insertions(+), 8 deletions(-)

Please add a test for the command and also add doc/usage

Regards,
Simon
Sughosh Ganu March 2, 2022, 4:56 a.m. UTC | #2
hi Simon,

On Tue, 1 Mar 2022 at 20:29, Simon Glass <sjg@chromium.org> wrote:
>
> Hi Sughosh,
>
> On Mon, 28 Feb 2022 at 05:07, 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 V1:
> > * Changed the help text to show order of the parameters passed
> >   to the rng command, based on review comment from Heinrich
> >
> >  cmd/rng.c | 31 +++++++++++++++++++++++--------
> >  1 file changed, 23 insertions(+), 8 deletions(-)
>
> Please add a test for the command and also add doc/usage

We already have a test for the RNG uclass, which is basically doing
the exact same thing. Do we still need one for the command? Or is it
for testing the command parameters?

-sughosh

>
> Regards,
> Simon
Simon Glass March 3, 2022, 3:47 a.m. UTC | #3
Hi Sughosh,

On Tue, 1 Mar 2022 at 21:56, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
>
> hi Simon,
>
> On Tue, 1 Mar 2022 at 20:29, Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Sughosh,
> >
> > On Mon, 28 Feb 2022 at 05:07, 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 V1:
> > > * Changed the help text to show order of the parameters passed
> > >   to the rng command, based on review comment from Heinrich
> > >
> > >  cmd/rng.c | 31 +++++++++++++++++++++++--------
> > >  1 file changed, 23 insertions(+), 8 deletions(-)
> >
> > Please add a test for the command and also add doc/usage
>
> We already have a test for the RNG uclass, which is basically doing
> the exact same thing. Do we still need one for the command? Or is it
> for testing the command parameters?

Yes and it provides test coverage. See acpi.c for an example test. It
can be quite simple I suspect.

Regards,
Simon
diff mbox series

Patch

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