[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 4/6] hw/misc: Add npcm7xx random number generator
From: |
Havard Skinnemoen |
Subject: |
Re: [PATCH 4/6] hw/misc: Add npcm7xx random number generator |
Date: |
Tue, 20 Oct 2020 16:40:09 -0700 |
On Tue, Oct 20, 2020 at 6:02 AM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Fri, 9 Oct 2020 at 00:22, Havard Skinnemoen <hskinnemoen@google.com> wrote:
> >
> > The RNG module returns a byte of randomness when the Data Valid bit is
> > set.
> >
> > This implementation ignores the prescaler setting, and loads a new value
> > into RNGD every time RNGCS is read while the RNG is enabled and random
> > data is available.
> >
> > A qtest featuring some simple randomness tests is included.
> >
> > Reviewed-by: Tyrone Ting <kfting@nuvoton.com>
> > Signed-off-by: Havard Skinnemoen <hskinnemoen@google.com>
>
>
> > +static const VMStateDescription vmstate_npcm7xx_rng = {
> > + .name = "npcm7xx-rng",
> > + .version_id = 0,
> > + .minimum_version_id = 0,
> > + .fields = (VMStateField[]) {
> > + VMSTATE_UINT8(rngcs, NPCM7xxRNGState),
> > + VMSTATE_UINT8(rngd, NPCM7xxRNGState),
> > + VMSTATE_UINT8(rngmode, NPCM7xxRNGState),
> > + },
>
> This is missing the VMSTATE_END_OF_LIST() terminator.
>
> > +};
> > +
>
> > --- /dev/null
> > +++ b/tests/qtest/npcm7xx_rng-test.c
> > @@ -0,0 +1,278 @@
> > +/*
> > + * QTest testcase for the Nuvoton NPCM7xx Random Number Generator
> > + *
> > + * Copyright 2020 Google LLC
> > + *
> > + * This program is free software; you can redistribute it and/or modify it
> > + * under the terms of the GNU General Public License as published by the
> > + * Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful, but
> > WITHOUT
> > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> > + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
> > + * for more details.
> > + */
> > +
> > +#include <math.h>
> > +
> > +#include "qemu/osdep.h"
>
> osdep.h must always be the first #include line, before anything
> else, including system includes.
>
> > + * Verifies that the first data byte collected after enabling the RNG
> > satisfies
> > + * a monobit test.
>
> Some of this "is this really a random number" testing seems a bit
> overkill given that the RNG device is just plumbing through the
> qemu_guest_getrandom() result, but since you've written the code
> we may as well keep it :-)
You may be right, though there were a couple of reasons why I added it.
One is that rngd was complaining about /dev/hwrng failing randomness
tests, so I wanted to make sure my device wasn't doing anything to
ruin the randomness. In the end, it turned out to be a kernel bug:
https://lkml.org/lkml/2020/9/23/1021
Another reason is that the value from qemu_guest_getrandom() isn't
consumed directly when reading the data register; it's latched when
reading the status register and returned on the next read from the
data register. So it's possible to imagine a bug causing the same
value to be returned over and over, which should cause the randomness
test to fail.
> If you fix the #include order and the missing terminator then
Will do, thanks!
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>
> thanks
> -- PMM
- [PATCH 0/6] Additional NPCM7xx features, devices and tests, Havard Skinnemoen, 2020/10/08
- [PATCH 1/6] tests/qtest: Add npcm7xx timer test, Havard Skinnemoen, 2020/10/08
- [PATCH 3/6] hw/timer: Adding watchdog for NPCM7XX Timer., Havard Skinnemoen, 2020/10/08
- [PATCH 5/6] hw/arm/npcm7xx: Add EHCI and OHCI controllers, Havard Skinnemoen, 2020/10/08
- [PATCH 4/6] hw/misc: Add npcm7xx random number generator, Havard Skinnemoen, 2020/10/08
- [PATCH 2/6] Move npcm7xx_timer_reached_zero call out of npcm7xx_timer_pause, Havard Skinnemoen, 2020/10/08
- [PATCH 6/6] hw/gpio: Add GPIO model for Nuvoton NPCM7xx, Havard Skinnemoen, 2020/10/08
- Re: [PATCH 0/6] Additional NPCM7xx features, devices and tests, Peter Maydell, 2020/10/20