On Sat, 10 May 2025 at 07:57, <jc...@duck.com> wrote:
>
> This patch implements the True Random Number Generator for the MAX78000 SOC
>
> Signed-off-by: Jackson Donaldson <jc...@duck.com>
> ---
> hw/arm/Kconfig | 1 +
> hw/arm/max78000_soc.c | 9 ++-
> hw/misc/Kconfig | 3 +
> hw/misc/max78000_gcr.c | 3 +
> hw/misc/max78000_trng.c | 115 ++++++++++++++++++++++++++++++++
> hw/misc/meson.build | 1 +
> include/hw/arm/max78000_soc.h | 2 +
> include/hw/misc/max78000_trng.h | 35 ++++++++++
> 8 files changed, 168 insertions(+), 1 deletion(-)
> create mode 100644 hw/misc/max78000_trng.c
> create mode 100644 include/hw/misc/max78000_trng.h
> +static uint64_t max78000_trng_read(void *opaque, hwaddr addr,
> + unsigned int size)
> +{
> + uint8_t buf[4];
> + Error *errp = NULL;
> +
> + Max78000TrngState *s = opaque;
> + switch (addr) {
> + case CTRL:{
> + return s->ctrl;
> + }
> + case STATUS:{
> + return 1;
> + }
> + case DATA:{
> + qcrypto_random_bytes(buf, 4, &errp);
> + return *(uint32_t *)buf;
> + }
No other code in hw/ calls qcrypto_random_bytes(). This kind
of "almost no other use of this function like this" is usually
a hint that it's not a good idea in new code. You want
qemu_guest_getrandom_nofail().
Also, instead of using a uint8_t buf[], use a
uint32_t and pass the address of that to the function:
uint32_t data;
qemu_guest_getrandom_nofail(&data, sizeof(data));
return data;
That avoids the need for a cast.
Your interrupt generation code in this device doesn't look
right: the interrupt is supposed to be generated when each
new random number is ready, so in our "generation takes
zero time" model a read from DATA should provoke a new
interrupt immediately (assuming the interrupt is enabled):
you need to simulate the ready status bit going low and
then high again.
See also my comments on an earlier patch about the usual
logic being to have an update function which does
"set interrupt to (condition && enabled)".
-- PMM
