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

[Havard Skinnemoen]

On Mon, Nov 2, 2020 at 3:36 AM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Fri, 23 Oct 2020 at 22:06, 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.
>
> > +int main(int argc, char **argv)
> > +{
> > +    int ret;
> > +
> > +    g_test_init(&argc, &argv, NULL);
> > +    g_test_set_nonfatal_assertions();
>
> While I was looking at this test case I noticed that it
> calls g_test_set_nonfatal_assertions(). Why does it do that?
> In our entire set of tests, only the npcm7xx test cases call
> that function, and they don't explain why they're a special
> case that needs to do so.

It's often useful to see more than the first failure when debugging
tests. Using the randomness flakiness as an example, it's very useful
to know if more than just one of the randomness tests fail. If I
remove g_test_set_nonfatal_assertions, I get:

**
ERROR:../../../tests/qtest/npcm7xx_rng-test.c:256:test_first_byte_runs:
assertion failed (calc_runs_p(buf.l, sizeof(buf) * BITS_PER_BYTE) >
0.01): (0.00204666737 > 0.01)
Bail out! 
ERROR:../../../tests/qtest/npcm7xx_rng-test.c:256:test_first_byte_runs:
assertion failed (calc_runs_p(buf.l, sizeof(buf) * BITS_PER_BYTE) >
0.01): (0.00204666737 > 0.01)
Aborted

which doesn't even tell me the name of the test that failed, let alone
anything about whether any subsequent tests passed.

Also note that it doesn't provide a clear "not ok" signal, which makes
life difficult for any downstream processing of the TAP output. It
looks like the test unexpectedly crashed.

Compare this to the output with g_test_set_nonfatal_assertions:

**
ERROR:../../../tests/qtest/npcm7xx_rng-test.c:232:test_first_byte_monobit:
assertion failed (calc_monobit_p(buf, sizeof(buf)) > 0.01):
(4.78548397e-05 > 0.01)
# ERROR:../../../tests/qtest/npcm7xx_rng-test.c:232:test_first_byte_monobit:
assertion failed (calc_monobit_p(buf, sizeof(buf)) > 0.01):
(4.78548397e-05 > 0.01)
not ok 5 /arm/npcm7xx_rng/first_byte/monobit
ok 6 /arm/npcm7xx_rng/first_byte/runs

which clearly shows that the "first_byte/monobit" test failed, and the
subsequent "first_byte/runs" test passed.

But none of this is really specific to the RNG test, so I can remove
it if you prefer for consistency.

Havard