On Sat, 25 Jul 2026 at 02:26, Marcelo Manzo <[email protected]> wrote:
>
> From: Sergey Kambalin <[email protected]>
>
> Add the BCM2838 (BCM2711/Raspberry Pi 4B) hardware random number
> generator: register model (control, FIFO data/count, int
> mask/status), a Fifo8-backed entropy buffer refilled from a
> qemu_rng backend on a ptimer tick, and interrupt generation once the
> FIFO crosses its configured threshold.
>
> Part of Sergey Kambalin's original Raspberry Pi 4B series (patchwork
> series 829638, posted to qemu-devel in 2024, never merged). Ported
> to current master by Marcelo Manzo: header paths moved under
> hw/core/, class_init()'s const void *data, the Resettable phases
> API, Property[] losing its DEFINE_PROP_END_OF_LIST() sentinel, and
> fifo8_pop_buf() gaining a caller-supplied destination buffer instead
> of returning an internal pointer (adapted bcm2838_rng200_read_fifo_data()
> to the new copying API, which QEMU's own fifo8.h now recommends over
> the old fifo8_pop_bufptr() this code originally used). No functional
> change otherwise.
>
> Boot-tested: guest probes it ("iproc-rng200 fe104000.rng: hwrng
> registered"), /dev/hwrng exists, and reading it returns real random
> bytes.
>
> Signed-off-by: Sergey Kambalin <[email protected]>
> Signed-off-by: Marcelo Manzo <[email protected]>
This patch mostly looks good, so the below is all minor stuff.
> +static const VMStateDescription vmstate_bcm2838_rng200 = {
> + .name = "bcm2838_rng200",
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .fields = (VMStateField[]) {
> + VMSTATE_UINT32(rng_fifo_cap, BCM2838Rng200State),
> + VMSTATE_ARRAY(regs, BCM2838Rng200State, N_BCM2838_RNG200_REGS, 0,
> + vmstate_info_uint32, uint32_t),
There's a VMSTATE_UINT32_ARRAY(), which is the simpler way to do this.
> +
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> +static bool is_rbg_enabled(BCM2838Rng200State *s)
> +{
> + return FIELD_EX32(s->regs[R_RNG_CTRL], RNG_CTRL, RBG_ENABLE);
> +}
> +
> +static void increment_bit_counter_by(BCM2838Rng200State *s, uint32_t inc_val)
> +{
> + s->regs[R_RNG_TOTAL_BIT_COUNT] += inc_val;
> +}
This function doesn't seem like it's really adding anything
compared to opencoding the addition in the one place we use it,
especially since that function's style otherwise is to open-code
s->regs[] accesses.
> diff --git a/hw/misc/trace-events b/hw/misc/trace-events
> index c9a868b3..90e178ed 100644
> --- a/hw/misc/trace-events
> +++ b/hw/misc/trace-events
> @@ -442,3 +442,13 @@ iommu_testdev_dma_read(uint64_t gva, uint32_t len)
> "gva=0x%" PRIx64 " len=%u"
> iommu_testdev_dma_verify(uint32_t expected, uint32_t actual) "expected=0x%x
> actual=0x%x"
> iommu_testdev_dma_result(uint32_t result) "DMA completed result=0x%x"
> iommu_testdev_dma_armed(bool armed) "armed=%d"
> +
> +# bcm2838_rng200.c
> +bcm2838_rng200_rng_soft_reset(void) "RNumG soft reset"
> +bcm2838_rng200_rbg_soft_reset(void) "RBitG soft reset"
> +bcm2838_rng200_enable_rbg(void) "RBitG enabled"
> +bcm2838_rng200_disable_rbg(void) "RBitG disabled"
We don't generally try to line the start of format strings up in trace-events
files (it doesn't work if you need to add something else later as you end up
either breaking the line-up or else having to edit more lines than just
the one you needed to change).
> +bcm2838_rng200_update_fifo(uint32_t len, uint32_t fifo_len) "len %u,
> fifo_len %u"
> +bcm2838_rng200_fifo_full(void) "RNumG FIFO full"
> +bcm2838_rng200_write(uint64_t addr, uint64_t value, unsigned size) "addr:
> 0x%"PRIx64" value: 0x%016" PRIx64 " size: %u"
> +bcm2838_rng200_read(uint64_t addr, unsigned size, uint64_t value) "addr:
> 0x%"PRIx64" size: %u value: 0x%016" PRIx64
> diff --git a/include/hw/misc/bcm2838_rng200.h
> b/include/hw/misc/bcm2838_rng200.h
> new file mode 100644
> index 00000000..05bc87fa
> --- /dev/null
> +++ b/include/hw/misc/bcm2838_rng200.h
> @@ -0,0 +1,42 @@
> +/*
> + * BCM2838 Random Number Generator emulation
> + *
> + * Copyright (C) 2022 Sergey Pushkarev <[email protected]>
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#ifndef BCM2838_RNG200_H
> +#define BCM2838_RNG200_H
> +
> +#include <stdbool.h>
Don't include standard headers -- osdep.h pulls them in for you, and
every .c file starts with including osdep.h, so every .h file has
them available too.
> +#include "qom/object.h"
> +#include "qemu/fifo8.h"
> +#include "system/rng.h"
> +#include "hw/core/sysbus.h"
> +#include "hw/core/ptimer.h"
> +#include "hw/core/qdev-clock.h"
> +#include "hw/core/irq.h"
> +
> +#define TYPE_BCM2838_RNG200 "bcm2838-rng200"
> +OBJECT_DECLARE_SIMPLE_TYPE(BCM2838Rng200State, BCM2838_RNG200)
> +
> +#define N_BCM2838_RNG200_REGS 10
> +
> +struct BCM2838Rng200State {
> + SysBusDevice busdev;
> + MemoryRegion iomem;
> +
> + ptimer_state *ptimer;
> + RngBackend *rng;
> + Clock *clock;
> +
> + uint32_t rng_fifo_cap;
> + Fifo8 fifo;
> +
> + qemu_irq irq;
> +
> + uint32_t regs[N_BCM2838_RNG200_REGS];
> +};
> +
> +#endif /* BCM2838_RNG200_H */
> --
> 2.47.1
thanks
-- PMM
