diff mbox series

[17/44] Add RNG200 skeleton

Message ID 20230726132512.149618-18-sergey.kambalin@auriga.com (mailing list archive)
State New, archived
Headers show
Series Raspberry Pi 4B machine | expand

Commit Message

Sergey Kambalin July 26, 2023, 1:24 p.m. UTC 
Signed-off-by: Sergey Kambalin <sergey.kambalin@auriga.com>
---
 hw/misc/bcm2838_rng200.c         | 118 +++++++++++++++++++++++++++++++
 hw/misc/meson.build              |   1 +
 hw/misc/trace-events             |  10 +++
 include/hw/misc/bcm2838_rng200.h |  77 ++++++++++++++++++++
 4 files changed, 206 insertions(+)
 create mode 100644 hw/misc/bcm2838_rng200.c
 create mode 100644 include/hw/misc/bcm2838_rng200.h

Comments

Peter Maydell Aug. 4, 2023, 1:25 p.m. UTC | #1 
On Wed, 26 Jul 2023 at 14:45, Sergey Kambalin <serg.oker@gmail.com> wrote:
>
> Signed-off-by: Sergey Kambalin <sergey.kambalin@auriga.com>
> ---
>  hw/misc/bcm2838_rng200.c         | 118 +++++++++++++++++++++++++++++++
>  hw/misc/meson.build              |   1 +
>  hw/misc/trace-events             |  10 +++
>  include/hw/misc/bcm2838_rng200.h |  77 ++++++++++++++++++++
>  4 files changed, 206 insertions(+)
>  create mode 100644 hw/misc/bcm2838_rng200.c
>  create mode 100644 include/hw/misc/bcm2838_rng200.h
>
> diff --git a/hw/misc/bcm2838_rng200.c b/hw/misc/bcm2838_rng200.c
> new file mode 100644
> index 0000000000..a17e8f2cda
> --- /dev/null
> +++ b/hw/misc/bcm2838_rng200.c
> @@ -0,0 +1,118 @@
> +/*
> + * BCM2838 Random Number Generator emulation
> + *
> + * Copyright (C) 2022 Sergey Pushkarev <sergey.pushkarev@auriga.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qapi/error.h"
> +#include "hw/qdev-properties.h"
> +#include "hw/misc/bcm2838_rng200.h"
> +#include "trace.h"
> +
> +static void bcm2838_rng200_rng_reset(BCM2838Rng200State *state)
> +{
> +    state->rng_ctrl.value = 0;
> +
> +    trace_bcm2838_rng200_rng_soft_reset();
> +}
> +
> +static uint64_t bcm2838_rng200_read(void *opaque, hwaddr offset,
> +                                    unsigned size)
> +{
> +    uint32_t res = 0;
> +
> +    trace_bcm2838_rng200_read((void *)offset, size, res);
> +    return res;
> +}
> +
> +static void bcm2838_rng200_write(void *opaque, hwaddr offset,
> +                                 uint64_t value, unsigned size)
> +{
> +
> +    trace_bcm2838_rng200_write((void *)offset, value, size);

Why trace the device instance pointer in these two tracepoints
and not in the others? (Generally we don't, unless there are
multiple devices of the same type in the system and it's important
to be able to distinguish them; and in that case there are
more useful ways to do it than the raw device pointer.)

> +}
> +
> +static const MemoryRegionOps bcm2838_rng200_ops = {
> +    .read = bcm2838_rng200_read,
> +    .write = bcm2838_rng200_write,
> +    .endianness = DEVICE_NATIVE_ENDIAN,
> +};
> +
> +static void bcm2838_rng200_realize(DeviceState *dev, Error **errp)
> +{
> +    BCM2838Rng200State *s = BCM2838_RNG200(dev);
> +
> +    if (s->rng == NULL) {
> +        Object *default_backend = object_new(TYPE_RNG_BUILTIN);
> +
> +        object_property_add_child(OBJECT(dev), "default-backend",
> +                                  default_backend);
> +        object_unref(default_backend);
> +
> +        object_property_set_link(OBJECT(dev), "rng", default_backend,
> +                                 errp);
> +    }
> +
> +    sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq);
> +}
> +
> +static void bcm2838_rng200_init(Object *obj)
> +{
> +    BCM2838Rng200State *s = BCM2838_RNG200(obj);
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
> +
> +    s->clock = qdev_init_clock_in(DEVICE(s), "rbg-clock",
> +                                  NULL, s,
> +                                  ClockPreUpdate);
> +    if (s->clock == NULL) {
> +        error_setg(&error_fatal, "Failed to init RBG clock");
> +        return;
> +    }
> +
> +    memory_region_init_io(&s->iomem, obj, &bcm2838_rng200_ops, s,
> +                          TYPE_BCM2838_RNG200, 0x28);
> +    sysbus_init_mmio(sbd, &s->iomem);
> +}
> +
> +static void bcm2838_rng200_reset(DeviceState *dev)
> +{
> +    BCM2838Rng200State *s = BCM2838_RNG200(dev);
> +    bcm2838_rng200_rng_reset(s);
> +}
> +
> +static Property bcm2838_rng200_properties[] = {
> +    DEFINE_PROP_UINT32("rbg-period", BCM2838Rng200State, rbg_period, 250),
> +    DEFINE_PROP_UINT32("rng-fifo-cap", BCM2838Rng200State, rng_fifo_cap, 128),
> +    DEFINE_PROP_LINK("rng", BCM2838Rng200State, rng,
> +                     TYPE_RNG_BACKEND, RngBackend *),
> +    DEFINE_PROP_BOOL("use-timer", BCM2838Rng200State, use_timer, true),

Do we really need all these properties? The SoC only has
one of these devices, so why do we need the flexibility ?

> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void bcm2838_rng200_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->realize = bcm2838_rng200_realize;
> +    dc->reset = bcm2838_rng200_reset;
> +    device_class_set_props(dc, bcm2838_rng200_properties);
> +}
> +
> +static const TypeInfo bcm2838_rng200_info = {
> +    .name          = TYPE_BCM2838_RNG200,
> +    .parent        = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(BCM2838Rng200State),
> +    .class_init    = bcm2838_rng200_class_init,
> +    .instance_init = bcm2838_rng200_init,
> +};
> +
> +static void bcm2838_rng200_register_types(void)
> +{
> +    type_register_static(&bcm2838_rng200_info);
> +}
> +
> +type_init(bcm2838_rng200_register_types)
> diff --git a/hw/misc/meson.build b/hw/misc/meson.build
> index 892f8b91c5..a6230ced43 100644
> --- a/hw/misc/meson.build
> +++ b/hw/misc/meson.build
> @@ -88,6 +88,7 @@ system_ss.add(when: 'CONFIG_RASPI', if_true: files(
>    'bcm2835_thermal.c',
>    'bcm2835_cprman.c',
>    'bcm2835_powermgt.c',
> +  'bcm2838_rng200.c'
>  ))
>  system_ss.add(when: 'CONFIG_SLAVIO', if_true: files('slavio_misc.c'))
>  system_ss.add(when: 'CONFIG_ZYNQ', if_true: files('zynq_slcr.c'))
> diff --git a/hw/misc/trace-events b/hw/misc/trace-events
> index 4d1a0e17af..d26cd2d22d 100644
> --- a/hw/misc/trace-events
> +++ b/hw/misc/trace-events
> @@ -297,3 +297,13 @@ virt_ctrl_instance_init(void *dev) "ctrl: %p"
>  lasi_chip_mem_valid(uint64_t addr, uint32_t val) "access to addr 0x%"PRIx64" is %d"
>  lasi_chip_read(uint64_t addr, uint32_t val) "addr 0x%"PRIx64" val 0x%08x"
>  lasi_chip_write(uint64_t addr, uint32_t val) "addr 0x%"PRIx64" val 0x%08x"
> +
> +# 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 ========="
> +bcm2838_rng200_update_fifo(uint32_t len, uint32_t fifo_len)    "len %u, fifo_len %u"
> +bcm2838_rng200_fifo_full(void) "========= RNumG FIFO FULL ========="

Can you make these trace strings a bit more in line with the style
for other trace strings, please (i.e. lose the caps and =======) ?

> +bcm2838_rng200_write(void *addr, uint64_t value, unsigned size) "addr %p, value 0x%016" PRIx64 ", size %u"
> +bcm2838_rng200_read(void *addr, unsigned size, uint64_t value) "addr %p, 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 0000000000..77f6cd8df4
> --- /dev/null
> +++ b/include/hw/misc/bcm2838_rng200.h
> @@ -0,0 +1,77 @@
> +/*
> + * BCM2838 Random Number Generator emulation
> + *
> + * Copyright (C) 2022 Sergey Pushkarev <sergey.pushkarev@auriga.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#ifndef BCM2838_RNG200_H
> +#define BCM2838_RNG200_H
> +
> +#include <stdbool.h>
> +#include "qom/object.h"
> +#include "qemu/fifo8.h"
> +#include "sysemu/rng.h"
> +#include "hw/sysbus.h"
> +#include "hw/ptimer.h"
> +#include "hw/qdev-clock.h"
> +#include "hw/irq.h"
> +
> +#define TYPE_BCM2838_RNG200 "bcm2838-rng200"
> +OBJECT_DECLARE_SIMPLE_TYPE(BCM2838Rng200State, BCM2838_RNG200)
> +
> +typedef union BCM2838Rng200Ctrl {
> +    uint32_t value;
> +    struct {
> +        uint32_t rbg_enable:1;
> +        uint32_t __r0:12;
> +        uint32_t div:8;
> +    };

Don't use bitfields in models of hardware (either of
register layouts or of in-guest-memory data), please.
It's not portable (bitfield layout depends on endianness,
plus on Windows the layout of structs with bitfields isn't
the same as GCC's normal standard).

Use normal-sized uint32_t values, and extract subfields
with bitfield ops. include/hw/registerfields.h has some
useful macros you can use where you define the fields in
a register, and then can get at them with eg
   bool enable = FIELD_EX32(s->ctrl, CTRL, ENABLE);

> +} BCM2838Rng200Ctrl;
> +
> +typedef union BCM2838Rng200Int {
> +    uint32_t value;
> +    struct {
> +        uint32_t total_bits_count_irq:1;
> +        uint32_t __r0:4;
> +        uint32_t nist_fail_irq:1;
> +        uint32_t __r1:11;
> +        uint32_t startup_transition_met_irq:1;
> +        uint32_t __r2:13;
> +        uint32_t master_fail_lockout_irq:1;
> +    };
> +} BCM2838Rng200Int;
> +
> +typedef union BCM2838Rng200FifoCount {
> +    uint32_t value;
> +    struct {
> +        uint32_t count:8;
> +        uint32_t thld:8;
> +    };
> +} BCM2838Rng200FifoCount;
> +
> +struct BCM2838Rng200State {
> +    SysBusDevice busdev;
> +    MemoryRegion iomem;
> +
> +    ptimer_state *ptimer;
> +    RngBackend *rng;
> +    Clock *clock;
> +
> +    uint32_t rbg_period;
> +    uint32_t rng_fifo_cap;
> +    bool use_timer;
> +
> +    Fifo8    fifo;
> +    qemu_irq irq;
> +    BCM2838Rng200Ctrl rng_ctrl;
> +    BCM2838Rng200Int rng_int_status;
> +    BCM2838Rng200Int rng_int_enable;
> +    uint32_t rng_total_bit_count;
> +    BCM2838Rng200FifoCount rng_fifo_count;
> +    uint32_t rng_bit_count_threshold;
> +};
> +
> +#endif /* BCM2838_RNG200_H */

As with the other device, my suggestion is to put the
vmstate struct definition in the same patch as the
fields are added to the device state struct.

thanks
-- PMM
diff mbox series

Patch

diff --git a/hw/misc/bcm2838_rng200.c b/hw/misc/bcm2838_rng200.c
new file mode 100644
index 0000000000..a17e8f2cda
--- /dev/null
+++ b/hw/misc/bcm2838_rng200.c
@@ -0,0 +1,118 @@ 
+/*
+ * BCM2838 Random Number Generator emulation
+ *
+ * Copyright (C) 2022 Sergey Pushkarev <sergey.pushkarev@auriga.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "hw/qdev-properties.h"
+#include "hw/misc/bcm2838_rng200.h"
+#include "trace.h"
+
+static void bcm2838_rng200_rng_reset(BCM2838Rng200State *state)
+{
+    state->rng_ctrl.value = 0;
+
+    trace_bcm2838_rng200_rng_soft_reset();
+}
+
+static uint64_t bcm2838_rng200_read(void *opaque, hwaddr offset,
+                                    unsigned size)
+{
+    uint32_t res = 0;
+
+    trace_bcm2838_rng200_read((void *)offset, size, res);
+    return res;
+}
+
+static void bcm2838_rng200_write(void *opaque, hwaddr offset,
+                                 uint64_t value, unsigned size)
+{
+
+    trace_bcm2838_rng200_write((void *)offset, value, size);
+}
+
+static const MemoryRegionOps bcm2838_rng200_ops = {
+    .read = bcm2838_rng200_read,
+    .write = bcm2838_rng200_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+static void bcm2838_rng200_realize(DeviceState *dev, Error **errp)
+{
+    BCM2838Rng200State *s = BCM2838_RNG200(dev);
+
+    if (s->rng == NULL) {
+        Object *default_backend = object_new(TYPE_RNG_BUILTIN);
+
+        object_property_add_child(OBJECT(dev), "default-backend",
+                                  default_backend);
+        object_unref(default_backend);
+
+        object_property_set_link(OBJECT(dev), "rng", default_backend,
+                                 errp);
+    }
+
+    sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq);
+}
+
+static void bcm2838_rng200_init(Object *obj)
+{
+    BCM2838Rng200State *s = BCM2838_RNG200(obj);
+    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
+
+    s->clock = qdev_init_clock_in(DEVICE(s), "rbg-clock",
+                                  NULL, s,
+                                  ClockPreUpdate);
+    if (s->clock == NULL) {
+        error_setg(&error_fatal, "Failed to init RBG clock");
+        return;
+    }
+
+    memory_region_init_io(&s->iomem, obj, &bcm2838_rng200_ops, s,
+                          TYPE_BCM2838_RNG200, 0x28);
+    sysbus_init_mmio(sbd, &s->iomem);
+}
+
+static void bcm2838_rng200_reset(DeviceState *dev)
+{
+    BCM2838Rng200State *s = BCM2838_RNG200(dev);
+    bcm2838_rng200_rng_reset(s);
+}
+
+static Property bcm2838_rng200_properties[] = {
+    DEFINE_PROP_UINT32("rbg-period", BCM2838Rng200State, rbg_period, 250),
+    DEFINE_PROP_UINT32("rng-fifo-cap", BCM2838Rng200State, rng_fifo_cap, 128),
+    DEFINE_PROP_LINK("rng", BCM2838Rng200State, rng,
+                     TYPE_RNG_BACKEND, RngBackend *),
+    DEFINE_PROP_BOOL("use-timer", BCM2838Rng200State, use_timer, true),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void bcm2838_rng200_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->realize = bcm2838_rng200_realize;
+    dc->reset = bcm2838_rng200_reset;
+    device_class_set_props(dc, bcm2838_rng200_properties);
+}
+
+static const TypeInfo bcm2838_rng200_info = {
+    .name          = TYPE_BCM2838_RNG200,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(BCM2838Rng200State),
+    .class_init    = bcm2838_rng200_class_init,
+    .instance_init = bcm2838_rng200_init,
+};
+
+static void bcm2838_rng200_register_types(void)
+{
+    type_register_static(&bcm2838_rng200_info);
+}
+
+type_init(bcm2838_rng200_register_types)
diff --git a/hw/misc/meson.build b/hw/misc/meson.build
index 892f8b91c5..a6230ced43 100644
--- a/hw/misc/meson.build
+++ b/hw/misc/meson.build
@@ -88,6 +88,7 @@  system_ss.add(when: 'CONFIG_RASPI', if_true: files(
   'bcm2835_thermal.c',
   'bcm2835_cprman.c',
   'bcm2835_powermgt.c',
+  'bcm2838_rng200.c'
 ))
 system_ss.add(when: 'CONFIG_SLAVIO', if_true: files('slavio_misc.c'))
 system_ss.add(when: 'CONFIG_ZYNQ', if_true: files('zynq_slcr.c'))
diff --git a/hw/misc/trace-events b/hw/misc/trace-events
index 4d1a0e17af..d26cd2d22d 100644
--- a/hw/misc/trace-events
+++ b/hw/misc/trace-events
@@ -297,3 +297,13 @@  virt_ctrl_instance_init(void *dev) "ctrl: %p"
 lasi_chip_mem_valid(uint64_t addr, uint32_t val) "access to addr 0x%"PRIx64" is %d"
 lasi_chip_read(uint64_t addr, uint32_t val) "addr 0x%"PRIx64" val 0x%08x"
 lasi_chip_write(uint64_t addr, uint32_t val) "addr 0x%"PRIx64" val 0x%08x"
+
+# 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 ========="
+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(void *addr, uint64_t value, unsigned size) "addr %p, value 0x%016" PRIx64 ", size %u"
+bcm2838_rng200_read(void *addr, unsigned size, uint64_t value) "addr %p, 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 0000000000..77f6cd8df4
--- /dev/null
+++ b/include/hw/misc/bcm2838_rng200.h
@@ -0,0 +1,77 @@ 
+/*
+ * BCM2838 Random Number Generator emulation
+ *
+ * Copyright (C) 2022 Sergey Pushkarev <sergey.pushkarev@auriga.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#ifndef BCM2838_RNG200_H
+#define BCM2838_RNG200_H
+
+#include <stdbool.h>
+#include "qom/object.h"
+#include "qemu/fifo8.h"
+#include "sysemu/rng.h"
+#include "hw/sysbus.h"
+#include "hw/ptimer.h"
+#include "hw/qdev-clock.h"
+#include "hw/irq.h"
+
+#define TYPE_BCM2838_RNG200 "bcm2838-rng200"
+OBJECT_DECLARE_SIMPLE_TYPE(BCM2838Rng200State, BCM2838_RNG200)
+
+typedef union BCM2838Rng200Ctrl {
+    uint32_t value;
+    struct {
+        uint32_t rbg_enable:1;
+        uint32_t __r0:12;
+        uint32_t div:8;
+    };
+} BCM2838Rng200Ctrl;
+
+typedef union BCM2838Rng200Int {
+    uint32_t value;
+    struct {
+        uint32_t total_bits_count_irq:1;
+        uint32_t __r0:4;
+        uint32_t nist_fail_irq:1;
+        uint32_t __r1:11;
+        uint32_t startup_transition_met_irq:1;
+        uint32_t __r2:13;
+        uint32_t master_fail_lockout_irq:1;
+    };
+} BCM2838Rng200Int;
+
+typedef union BCM2838Rng200FifoCount {
+    uint32_t value;
+    struct {
+        uint32_t count:8;
+        uint32_t thld:8;
+    };
+} BCM2838Rng200FifoCount;
+
+struct BCM2838Rng200State {
+    SysBusDevice busdev;
+    MemoryRegion iomem;
+
+    ptimer_state *ptimer;
+    RngBackend *rng;
+    Clock *clock;
+
+    uint32_t rbg_period;
+    uint32_t rng_fifo_cap;
+    bool use_timer;
+
+    Fifo8    fifo;
+    qemu_irq irq;
+    BCM2838Rng200Ctrl rng_ctrl;
+    BCM2838Rng200Int rng_int_status;
+    BCM2838Rng200Int rng_int_enable;
+    uint32_t rng_total_bit_count;
+    BCM2838Rng200FifoCount rng_fifo_count;
+    uint32_t rng_bit_count_threshold;
+};
+
+#endif /* BCM2838_RNG200_H */