Re: [PATCH 4/6] hw/misc: Add npcm7xx random number generator

[Peter Maydell]

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

If you fix the #include order and the missing terminator then

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM