qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/3] AMD/Xilinx Versal TRNG support
@ 2023-10-17 19:32 Tong Ho
  2023-10-17 19:32 ` [PATCH v4 1/3] hw/misc: Introduce AMD/Xilix Versal TRNG device Tong Ho
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Tong Ho @ 2023-10-17 19:32 UTC (permalink / raw)
  To: qemu-arm
  Cc: qemu-devel, alistair, edgar.iglesias, peter.maydell,
	richard.henderson, frasse.iglesias, tong.ho

This series adds support for the True Random Number Generator
(TRNG) in the AMD/Xilinx Versal family of devices.

The series starts by introducing a non-cryptographic grade model
of the TRNG controller in the Versal family of devices, followed
by instantiating the model in Xilinx Versal machine.

The series ends with a q-test for sanity check of the TRNG model
in the Xilinx Versal machine.

V3 => V4
1) Patch #1
   a) Simplify the data given to g_rand_set_seed_array() as an
      array of guint32.
   b) Use qemu_guest_getrandom_nofail() as the entropy source
      when the device is configured in TRNG (mode 3).
   c) Allow 0 and (2^32-1) as valid output of the generator
   d) Add output-related context to VMSTATE.
2) Patch #3
   Remove test's assumption of PRNG output != TRNG output.

V2 => V3
1) Patch #1 Use DEFINE_PROP to define a property with custom action on set
2) Patch #1 Remove duplicated version-check on writing to CTRL4 register
3) Patch #2 Defer adding TRNG to machine's device-tree to a future patch
4) Patch #3 Code style fix: group all local vars definitions together

V1 => V2
1) Change patch #1 only
2) Use g_rand_*() PRNG from glib to replace V1's custom PRNG.
3) Implement ResettableClass for device-reset.
4) Add device-mode description to commit-message.

Best regards,
Tong Ho

Tong Ho (3):
  hw/misc: Introduce AMD/Xilix Versal TRNG device
  hw/arm: xlnx-versal-virt: Add AMD/Xilinx TRNG device
  tests/qtest: Introduce tests for AMD/Xilinx Versal TRNG device

 hw/arm/Kconfig                      |   1 +
 hw/arm/xlnx-versal.c                |  16 +
 hw/misc/Kconfig                     |   3 +
 hw/misc/meson.build                 |   3 +
 hw/misc/xlnx-versal-trng.c          | 727 ++++++++++++++++++++++++++++
 include/hw/arm/xlnx-versal.h        |   5 +
 include/hw/misc/xlnx-versal-trng.h  |  58 +++
 tests/qtest/meson.build             |   2 +-
 tests/qtest/xlnx-versal-trng-test.c | 486 +++++++++++++++++++
 9 files changed, 1300 insertions(+), 1 deletion(-)
 create mode 100644 hw/misc/xlnx-versal-trng.c
 create mode 100644 include/hw/misc/xlnx-versal-trng.h
 create mode 100644 tests/qtest/xlnx-versal-trng-test.c

-- 
2.25.1



^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v4 1/3] hw/misc: Introduce AMD/Xilix Versal TRNG device
  2023-10-17 19:32 [PATCH v4 0/3] AMD/Xilinx Versal TRNG support Tong Ho
@ 2023-10-17 19:32 ` Tong Ho
  2023-10-27 12:54   ` Peter Maydell
  2023-10-17 19:32 ` [PATCH v4 2/3] hw/arm: xlnx-versal-virt: Add AMD/Xilinx " Tong Ho
  2023-10-17 19:32 ` [PATCH v4 3/3] tests/qtest: Introduce tests for AMD/Xilinx Versal " Tong Ho
  2 siblings, 1 reply; 10+ messages in thread
From: Tong Ho @ 2023-10-17 19:32 UTC (permalink / raw)
  To: qemu-arm
  Cc: qemu-devel, alistair, edgar.iglesias, peter.maydell,
	richard.henderson, frasse.iglesias, tong.ho

This adds a non-cryptographic grade implementation of the
model for the True Random Number Generator (TRNG) component
in AMD/Xilinx Versal device family.

This implements all 3 modes defined by the actual hardware
specs, all of which selectable by guest software at will
at anytime:
1) PRNG mode, in which the generated sequence is required to
   be reproducible after reseeded by the same 384-bit value
   as supplied by guest software.
2) Test mode, in which the generated sequence is required to
   be reproducible ater reseeded by the same 128-bit test
   seed supplied by guest software.
3) TRNG mode, in which non-reproducible sequence is generated
   based on periodic reseed by a suitable entropy source.

This model is only intended for non-real world testing of
guest software, where cryptographically strong PRNG or TRNG
is not needed.

This model supports versions 1 & 2 of the device, with
default to be version 2; the 'hw-version' uint32 property
can be set to 0x0100 to override the default.

Other implemented properties:
- 'forced-prng', uint64
  When set to non-zero, mode 3's entropy source is implemented
  as a deterministic sequence based on the given value and other
  deterministic parameters.
  This option allows the emulation to test guest software using
  mode 3 and to reproduce data-dependent defects.

- 'fips-fault-events', uint32, bit-mask
  bit 3: Triggers the SP800-90B entropy health test fault irq
  bit 1: Triggers the FIPS 140-2 continuous test fault irq

Signed-off-by: Tong Ho <tong.ho@amd.com>
---
 hw/misc/Kconfig                    |   3 +
 hw/misc/meson.build                |   3 +
 hw/misc/xlnx-versal-trng.c         | 727 +++++++++++++++++++++++++++++
 include/hw/misc/xlnx-versal-trng.h |  58 +++
 4 files changed, 791 insertions(+)
 create mode 100644 hw/misc/xlnx-versal-trng.c
 create mode 100644 include/hw/misc/xlnx-versal-trng.h

diff --git a/hw/misc/Kconfig b/hw/misc/Kconfig
index dba41afe67..cc8a8c1418 100644
--- a/hw/misc/Kconfig
+++ b/hw/misc/Kconfig
@@ -197,4 +197,7 @@ config DJMEMC
 config IOSB
     bool
 
+config XLNX_VERSAL_TRNG
+    bool
+
 source macio/Kconfig
diff --git a/hw/misc/meson.build b/hw/misc/meson.build
index f60de33f9a..36c20d5637 100644
--- a/hw/misc/meson.build
+++ b/hw/misc/meson.build
@@ -104,6 +104,9 @@ system_ss.add(when: 'CONFIG_XLNX_VERSAL', if_true: files(
   'xlnx-cfi-if.c',
   'xlnx-versal-cframe-reg.c',
 ))
+system_ss.add(when: 'CONFIG_XLNX_VERSAL_TRNG', if_true: files(
+  'xlnx-versal-trng.c',
+))
 system_ss.add(when: 'CONFIG_STM32F2XX_SYSCFG', if_true: files('stm32f2xx_syscfg.c'))
 system_ss.add(when: 'CONFIG_STM32F4XX_SYSCFG', if_true: files('stm32f4xx_syscfg.c'))
 system_ss.add(when: 'CONFIG_STM32F4XX_EXTI', if_true: files('stm32f4xx_exti.c'))
diff --git a/hw/misc/xlnx-versal-trng.c b/hw/misc/xlnx-versal-trng.c
new file mode 100644
index 0000000000..fb0130a6b2
--- /dev/null
+++ b/hw/misc/xlnx-versal-trng.c
@@ -0,0 +1,727 @@
+/*
+ * Non-crypto strength model of the True Random Number Generator
+ * in the AMD/Xilinx Versal device family.
+ *
+ * Copyright (c) 2017-2020 Xilinx Inc.
+ * Copyright (c) 2023 Advanced Micro Devices, Inc.
+ *
+ * Written by Edgar E. Iglesias <edgar.iglesias@xilinx.com>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+#include "qemu/osdep.h"
+#include "hw/misc/xlnx-versal-trng.h"
+
+#include "qemu/bitops.h"
+#include "qemu/log.h"
+#include "qemu/error-report.h"
+#include "qemu/guest-random.h"
+#include "qemu/timer.h"
+#include "qapi/visitor.h"
+#include "migration/vmstate.h"
+#include "hw/qdev-properties.h"
+
+#ifndef XLNX_VERSAL_TRNG_ERR_DEBUG
+#define XLNX_VERSAL_TRNG_ERR_DEBUG 0
+#endif
+
+REG32(INT_CTRL, 0x0)
+    FIELD(INT_CTRL, CERTF_RST, 5, 1)
+    FIELD(INT_CTRL, DTF_RST, 4, 1)
+    FIELD(INT_CTRL, DONE_RST, 3, 1)
+    FIELD(INT_CTRL, CERTF_EN, 2, 1)
+    FIELD(INT_CTRL, DTF_EN, 1, 1)
+    FIELD(INT_CTRL, DONE_EN, 0, 1)
+REG32(STATUS, 0x4)
+    FIELD(STATUS, QCNT, 9, 3)
+    FIELD(STATUS, EAT, 4, 5)
+    FIELD(STATUS, CERTF, 3, 1)
+    FIELD(STATUS, DTF, 1, 1)
+    FIELD(STATUS, DONE, 0, 1)
+REG32(CTRL, 0x8)
+    FIELD(CTRL, PERSODISABLE, 10, 1)
+    FIELD(CTRL, SINGLEGENMODE, 9, 1)
+    FIELD(CTRL, EUMODE, 8, 1)
+    FIELD(CTRL, PRNGMODE, 7, 1)
+    FIELD(CTRL, TSTMODE, 6, 1)
+    FIELD(CTRL, PRNGSTART, 5, 1)
+    FIELD(CTRL, EATAU, 4, 1)
+    FIELD(CTRL, PRNGXS, 3, 1)
+    FIELD(CTRL, TRSSEN, 2, 1)
+    FIELD(CTRL, QERTUEN, 1, 1)
+    FIELD(CTRL, PRNGSRST, 0, 1)
+REG32(CTRL_2, 0xc)
+    FIELD(CTRL_2, REPCOUNTTESTCUTOFF, 8, 9)
+    FIELD(CTRL_2, RESERVED_7_5, 5, 3)
+    FIELD(CTRL_2, DIT, 0, 5)
+REG32(CTRL_3, 0x10)
+    FIELD(CTRL_3, ADAPTPROPTESTCUTOFF, 8, 10)
+    FIELD(CTRL_3, DLEN, 0, 8)
+REG32(CTRL_4, 0x14)
+    FIELD(CTRL_4, SINGLEBITRAW, 0, 1)
+REG32(EXT_SEED_0, 0x40)
+REG32(EXT_SEED_1, 0x44)
+REG32(EXT_SEED_2, 0x48)
+REG32(EXT_SEED_3, 0x4c)
+REG32(EXT_SEED_4, 0x50)
+REG32(EXT_SEED_5, 0x54)
+REG32(EXT_SEED_6, 0x58)
+REG32(EXT_SEED_7, 0x5c)
+REG32(EXT_SEED_8, 0x60)
+REG32(EXT_SEED_9, 0x64)
+REG32(EXT_SEED_10, 0x68)
+REG32(EXT_SEED_11, 0x6c)
+REG32(PER_STRNG_0, 0x80)
+REG32(PER_STRNG_1, 0x84)
+REG32(PER_STRNG_2, 0x88)
+REG32(PER_STRNG_3, 0x8c)
+REG32(PER_STRNG_4, 0x90)
+REG32(PER_STRNG_5, 0x94)
+REG32(PER_STRNG_6, 0x98)
+REG32(PER_STRNG_7, 0x9c)
+REG32(PER_STRNG_8, 0xa0)
+REG32(PER_STRNG_9, 0xa4)
+REG32(PER_STRNG_10, 0xa8)
+REG32(PER_STRNG_11, 0xac)
+REG32(CORE_OUTPUT, 0xc0)
+REG32(RESET, 0xd0)
+    FIELD(RESET, VAL, 0, 1)
+REG32(OSC_EN, 0xd4)
+    FIELD(OSC_EN, VAL, 0, 1)
+REG32(TRNG_ISR, 0xe0)
+    FIELD(TRNG_ISR, SLVERR, 1, 1)
+    FIELD(TRNG_ISR, CORE_INT, 0, 1)
+REG32(TRNG_IMR, 0xe4)
+    FIELD(TRNG_IMR, SLVERR, 1, 1)
+    FIELD(TRNG_IMR, CORE_INT, 0, 1)
+REG32(TRNG_IER, 0xe8)
+    FIELD(TRNG_IER, SLVERR, 1, 1)
+    FIELD(TRNG_IER, CORE_INT, 0, 1)
+REG32(TRNG_IDR, 0xec)
+    FIELD(TRNG_IDR, SLVERR, 1, 1)
+    FIELD(TRNG_IDR, CORE_INT, 0, 1)
+REG32(SLV_ERR_CTRL, 0xf0)
+    FIELD(SLV_ERR_CTRL, ENABLE, 0, 1)
+
+#define R_MAX (R_SLV_ERR_CTRL + 1)
+
+QEMU_BUILD_BUG_ON(R_MAX * 4 != sizeof_field(XlnxVersalTRng, regs));
+
+#define TRNG_GUEST_ERROR(D, FMT, ...) \
+    do {                                                               \
+        g_autofree char *p = object_get_canonical_path(OBJECT(D));     \
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: " FMT, p, ## __VA_ARGS__); \
+    } while (0)
+
+#define TRNG_WARN(D, FMT, ...) \
+    do {                                                               \
+        g_autofree char *p = object_get_canonical_path(OBJECT(D));     \
+        warn_report("%s: " FMT, p, ## __VA_ARGS__);                    \
+    } while (0)
+
+static bool trng_older_than_v2(XlnxVersalTRng *s)
+{
+    return s->hw_version < 0x0200;
+}
+
+static bool trng_in_reset(XlnxVersalTRng *s)
+{
+    if (ARRAY_FIELD_EX32(s->regs, RESET, VAL)) {
+        return true;
+    }
+    if (ARRAY_FIELD_EX32(s->regs, CTRL, PRNGSRST)) {
+        return true;
+    }
+
+    return false;
+}
+
+static bool trng_test_enabled(XlnxVersalTRng *s)
+{
+    return ARRAY_FIELD_EX32(s->regs, CTRL, TSTMODE);
+}
+
+static bool trng_trss_enabled(XlnxVersalTRng *s)
+{
+    if (trng_in_reset(s)) {
+        return false;
+    }
+    if (!ARRAY_FIELD_EX32(s->regs, CTRL, TRSSEN)) {
+        return false;
+    }
+    if (!ARRAY_FIELD_EX32(s->regs, OSC_EN, VAL)) {
+        return false;
+    }
+
+    return true;
+}
+
+static void trng_le128(uint32_t *le128, uint64_t h00, uint64_t h64)
+{
+    le128[0] = cpu_to_le32(h00 & 0xffffffffU);
+    le128[1] = cpu_to_le32(h00 >> 32);
+    le128[2] = cpu_to_le32(h64 & 0xffffffffU);
+    le128[3] = cpu_to_le32(h64 >> 32);
+}
+
+static void trng_le384(uint32_t *le384, const uint32_t *h384)
+{
+    size_t i;
+
+    for (i = 0; i < (384 / 32); i++) {
+        le384[i] = cpu_to_le32(h384[i]);
+    }
+}
+
+static void trng_reseed(XlnxVersalTRng *s)
+{
+    bool ext_seed = ARRAY_FIELD_EX32(s->regs, CTRL, PRNGXS);
+    bool pers_disabled = ARRAY_FIELD_EX32(s->regs, CTRL, PERSODISABLE);
+
+    enum {
+        U384_U32 = 384 / 32,
+    };
+
+    /*
+     * Maximum seed length is len(personalized string) + len(ext seed).
+     *
+     * Use little-endian to ensure guest sequence being indepedent of
+     * host endian.
+     */
+    guint32 gs[U384_U32 * 2], *seed = &gs[U384_U32];
+
+    /*
+     * A disabled personalized string is the same as
+     * a string with all zeros.
+     *
+     * The device's hardware spec defines 3 modes (all selectable
+     * by guest at will and at anytime):
+     * 1) External seeding
+     *    This is a PRNG mode, in which the produced sequence shall
+     *    be reproducible if reseeded by the same 384-bit seed, as
+     *    supplied by guest software.
+     * 2) Test seeding
+     *    This is a PRNG mode, in which the produced sequence shall
+     *    be reproducible if reseeded by a 128-bit test seed, as
+     *    supplied by guest software.
+     * 3) Truly-random seeding
+     *    This is the TRNG mode, in which the produced sequence is
+     *    periodically reseeded by a crypto-strength entropy source.
+     *
+     * To assist debugging of certain classes of software defects,
+     * this QEMU model implements a 4th mode,
+     * 4) Forced PRNG
+     *    When in this mode, a reproducible sequence is generated
+     *    if software has selected the TRNG mode (mode 2).
+     *
+     *    This emulation-only mode can only be selected by setting
+     *    the uint64 property 'forced-prng' to a non-zero value.
+     *    Guest software cannot select this mode.
+     */
+    memset(gs, 0, sizeof(gs));
+    stb_p((uint8_t *)gs + sizeof(gs) - 1, 1);
+
+    if (!pers_disabled) {
+        trng_le384(gs, &s->regs[R_PER_STRNG_0]);
+    }
+
+    if (ext_seed) {
+        trng_le384(seed, &s->regs[R_EXT_SEED_0]);
+    } else if (trng_test_enabled(s)) {
+        trng_le128(seed, s->tst_seed[0], s->tst_seed[1]);
+    } else if (s->forced_prng_seed) {
+        s->forced_prng_count++;
+        trng_le128(seed, s->forced_prng_count, s->forced_prng_seed);
+    } else {
+        qemu_guest_getrandom_nofail(seed, U384_U32 * 4);
+    }
+
+    g_rand_set_seed_array(s->prng, gs, ARRAY_SIZE(gs));
+
+    s->rand_count = 0;
+    s->rand_reseed = 1ULL << 48;
+}
+
+static void trng_regen(XlnxVersalTRng *s)
+{
+    if (s->rand_reseed == 0) {
+        TRNG_GUEST_ERROR(s, "Too many generations without a reseed");
+        trng_reseed(s);
+    }
+    s->rand_reseed--;
+
+    /*
+     * In real hardware, each regen creates 256 bits, but QCNT
+     * reports a max of 4.
+     */
+    ARRAY_FIELD_DP32(s->regs, STATUS, QCNT, 4);
+    s->rand_count = 256 / 32;
+}
+
+static uint32_t trng_rdout(XlnxVersalTRng *s)
+{
+    assert(s->rand_count);
+
+    s->rand_count--;
+    if (s->rand_count < 4) {
+        ARRAY_FIELD_DP32(s->regs, STATUS, QCNT, s->rand_count);
+    }
+
+    return g_rand_int(s->prng);
+}
+
+static void trng_irq_update(XlnxVersalTRng *s)
+{
+    bool pending = s->regs[R_TRNG_ISR] & ~s->regs[R_TRNG_IMR];
+    qemu_set_irq(s->irq, pending);
+}
+
+static void trng_isr_postw(RegisterInfo *reg, uint64_t val64)
+{
+    XlnxVersalTRng *s = XLNX_VERSAL_TRNG(reg->opaque);
+    trng_irq_update(s);
+}
+
+static uint64_t trng_ier_prew(RegisterInfo *reg, uint64_t val64)
+{
+    XlnxVersalTRng *s = XLNX_VERSAL_TRNG(reg->opaque);
+    uint32_t val = val64;
+
+    s->regs[R_TRNG_IMR] &= ~val;
+    trng_irq_update(s);
+    return 0;
+}
+
+static uint64_t trng_idr_prew(RegisterInfo *reg, uint64_t val64)
+{
+    XlnxVersalTRng *s = XLNX_VERSAL_TRNG(reg->opaque);
+    uint32_t val = val64;
+
+    s->regs[R_TRNG_IMR] |= val;
+    trng_irq_update(s);
+    return 0;
+}
+
+static void trng_core_int_update(XlnxVersalTRng *s)
+{
+    bool pending = false;
+    uint32_t st = s->regs[R_STATUS];
+    uint32_t en = s->regs[R_INT_CTRL];
+
+    if (FIELD_EX32(st, STATUS, CERTF) && FIELD_EX32(en, INT_CTRL, CERTF_EN)) {
+        pending = true;
+    }
+
+    if (FIELD_EX32(st, STATUS, DTF) && FIELD_EX32(en, INT_CTRL, DTF_EN)) {
+        pending = true;
+    }
+
+    if (FIELD_EX32(st, STATUS, DONE) && FIELD_EX32(en, INT_CTRL, DONE_EN)) {
+        pending = true;
+    }
+
+    ARRAY_FIELD_DP32(s->regs, TRNG_ISR, CORE_INT, pending);
+    trng_irq_update(s);
+}
+
+static void trng_int_ctrl_postw(RegisterInfo *reg, uint64_t val64)
+{
+    XlnxVersalTRng *s = XLNX_VERSAL_TRNG(reg->opaque);
+    uint32_t v32 = val64;
+    uint32_t clr_mask = 0;
+
+    if (FIELD_EX32(v32, INT_CTRL, CERTF_RST)) {
+        clr_mask |= R_STATUS_CERTF_MASK;
+    }
+    if (FIELD_EX32(v32, INT_CTRL, DTF_RST)) {
+        clr_mask |= R_STATUS_DTF_MASK;
+    }
+    if (FIELD_EX32(v32, INT_CTRL, DONE_RST)) {
+        clr_mask |= R_STATUS_DONE_MASK;
+    }
+
+    s->regs[R_STATUS] &= ~clr_mask;
+    trng_core_int_update(s);
+}
+
+static void trng_done(XlnxVersalTRng *s)
+{
+    ARRAY_FIELD_DP32(s->regs, STATUS, DONE, true);
+    trng_core_int_update(s);
+}
+
+static void trng_fault_event_set(XlnxVersalTRng *s, uint32_t events)
+{
+    bool pending = false;
+
+    /* Disabled TRSS cannot generate any fault event */
+    if (!trng_trss_enabled(s)) {
+        return;
+    }
+
+    if (FIELD_EX32(events, STATUS, CERTF)) {
+        /* In older version, ERTU must be enabled explicitly to get CERTF */
+        if (trng_older_than_v2(s) &&
+            !ARRAY_FIELD_EX32(s->regs, CTRL, QERTUEN)) {
+            TRNG_WARN(s, "CERTF injection ignored: ERTU disabled");
+        } else {
+            ARRAY_FIELD_DP32(s->regs, STATUS, CERTF, true);
+            pending = true;
+        }
+    }
+
+    if (FIELD_EX32(events, STATUS, DTF)) {
+        ARRAY_FIELD_DP32(s->regs, STATUS, DTF, true);
+        pending = true;
+    }
+
+    if (pending) {
+        trng_core_int_update(s);
+    }
+}
+
+static void trng_soft_reset(XlnxVersalTRng *s)
+{
+    s->rand_count = 0;
+    s->regs[R_STATUS] = 0;
+
+    ARRAY_FIELD_DP32(s->regs, TRNG_ISR, CORE_INT, 0);
+}
+
+static void trng_ctrl_postw(RegisterInfo *reg, uint64_t val64)
+{
+    XlnxVersalTRng *s = XLNX_VERSAL_TRNG(reg->opaque);
+
+    if (trng_in_reset(s)) {
+        return;
+    }
+
+    if (FIELD_EX32(val64, CTRL, PRNGSRST)) {
+        trng_soft_reset(s);
+        trng_irq_update(s);
+        return;
+    }
+
+    if (!FIELD_EX32(val64, CTRL, PRNGSTART)) {
+        return;
+    }
+
+    if (FIELD_EX32(val64, CTRL, PRNGMODE)) {
+        trng_regen(s);
+    } else {
+        trng_reseed(s);
+    }
+
+    trng_done(s);
+}
+
+static void trng_ctrl4_postw(RegisterInfo *reg, uint64_t val64)
+{
+    XlnxVersalTRng *s = XLNX_VERSAL_TRNG(reg->opaque);
+
+    /* Only applies to test mode with TRSS enabled */
+    if (!trng_test_enabled(s) || !trng_trss_enabled(s)) {
+        return;
+    }
+
+    /* Shift in a single bit.  */
+    s->tst_seed[1] <<= 1;
+    s->tst_seed[1] |= s->tst_seed[0] >> 63;
+    s->tst_seed[0] <<= 1;
+    s->tst_seed[0] |= val64 & 1;
+
+    trng_reseed(s);
+    trng_regen(s);
+}
+
+static uint64_t trng_core_out_postr(RegisterInfo *reg, uint64_t val)
+{
+    XlnxVersalTRng *s = XLNX_VERSAL_TRNG(reg->opaque);
+    bool oneshot = ARRAY_FIELD_EX32(s->regs, CTRL, SINGLEGENMODE);
+    bool start = ARRAY_FIELD_EX32(s->regs, CTRL, PRNGSTART);
+    uint32_t r = 0xbad;
+
+    if (trng_in_reset(s)) {
+        TRNG_GUEST_ERROR(s, "Reading random number while in reset!");
+        return r;
+    }
+
+    if (s->rand_count == 0) {
+        TRNG_GUEST_ERROR(s, "Reading random number when unavailable!");
+        return r;
+    }
+
+    r = trng_rdout(s);
+
+    /* Automatic mode regenerates when half the output reg is empty.  */
+    if (!oneshot && start && s->rand_count <= 3) {
+        trng_regen(s);
+    }
+
+    return r;
+}
+
+static void trng_reset(XlnxVersalTRng *s)
+{
+    unsigned int i;
+
+    s->forced_prng_count = 0;
+
+    for (i = 0; i < ARRAY_SIZE(s->regs_info); ++i) {
+        register_reset(&s->regs_info[i]);
+    }
+    trng_soft_reset(s);
+    trng_irq_update(s);
+}
+
+static uint64_t trng_reset_prew(RegisterInfo *reg, uint64_t val64)
+{
+    XlnxVersalTRng *s = XLNX_VERSAL_TRNG(reg->opaque);
+
+    if (!ARRAY_FIELD_EX32(s->regs, RESET, VAL) &&
+        FIELD_EX32(val64, RESET, VAL)) {
+        trng_reset(s);
+    }
+
+    return val64;
+}
+
+static uint64_t trng_register_read(void *opaque, hwaddr addr, unsigned size)
+{
+    /*
+     * Guest provided seed and personalized strings cannot be
+     * read back, and read attempts return value of A_STATUS.
+     */
+    switch (addr) {
+    case A_EXT_SEED_0 ... A_PER_STRNG_11:
+        addr = A_STATUS;
+        break;
+    }
+
+    return register_read_memory(opaque, addr, size);
+}
+
+static void trng_register_write(void *opaque, hwaddr addr,
+                                uint64_t value, unsigned size)
+{
+    RegisterInfoArray *reg_array = opaque;
+    XlnxVersalTRng *s = XLNX_VERSAL_TRNG(reg_array->r[0]->opaque);
+
+    if (trng_older_than_v2(s)) {
+        switch (addr) {
+        case A_CTRL:
+            value = FIELD_DP64(value, CTRL, PERSODISABLE, 0);
+            value = FIELD_DP64(value, CTRL, SINGLEGENMODE, 0);
+            break;
+        case A_CTRL_2:
+        case A_CTRL_3:
+        case A_CTRL_4:
+            return;
+        }
+    } else {
+        switch (addr) {
+        case A_CTRL:
+            value = FIELD_DP64(value, CTRL, EATAU, 0);
+            value = FIELD_DP64(value, CTRL, QERTUEN, 0);
+            break;
+        }
+    }
+
+    register_write_memory(opaque, addr, value, size);
+}
+
+static RegisterAccessInfo trng_regs_info[] = {
+    {   .name = "INT_CTRL",  .addr = A_INT_CTRL,
+        .post_write = trng_int_ctrl_postw,
+    },{ .name = "STATUS",  .addr = A_STATUS,
+        .ro = 0xfff,
+    },{ .name = "CTRL",  .addr = A_CTRL,
+        .post_write = trng_ctrl_postw,
+    },{ .name = "CTRL_2",  .addr = A_CTRL_2,
+        .reset = 0x210c,
+    },{ .name = "CTRL_3",  .addr = A_CTRL_3,
+        .reset = 0x26f09,
+    },{ .name = "CTRL_4",  .addr = A_CTRL_4,
+        .post_write = trng_ctrl4_postw,
+    },{ .name = "EXT_SEED_0",  .addr = A_EXT_SEED_0,
+    },{ .name = "EXT_SEED_1",  .addr = A_EXT_SEED_1,
+    },{ .name = "EXT_SEED_2",  .addr = A_EXT_SEED_2,
+    },{ .name = "EXT_SEED_3",  .addr = A_EXT_SEED_3,
+    },{ .name = "EXT_SEED_4",  .addr = A_EXT_SEED_4,
+    },{ .name = "EXT_SEED_5",  .addr = A_EXT_SEED_5,
+    },{ .name = "EXT_SEED_6",  .addr = A_EXT_SEED_6,
+    },{ .name = "EXT_SEED_7",  .addr = A_EXT_SEED_7,
+    },{ .name = "EXT_SEED_8",  .addr = A_EXT_SEED_8,
+    },{ .name = "EXT_SEED_9",  .addr = A_EXT_SEED_9,
+    },{ .name = "EXT_SEED_10",  .addr = A_EXT_SEED_10,
+    },{ .name = "EXT_SEED_11",  .addr = A_EXT_SEED_11,
+    },{ .name = "PER_STRNG_0",  .addr = A_PER_STRNG_0,
+    },{ .name = "PER_STRNG_1",  .addr = A_PER_STRNG_1,
+    },{ .name = "PER_STRNG_2",  .addr = A_PER_STRNG_2,
+    },{ .name = "PER_STRNG_3",  .addr = A_PER_STRNG_3,
+    },{ .name = "PER_STRNG_4",  .addr = A_PER_STRNG_4,
+    },{ .name = "PER_STRNG_5",  .addr = A_PER_STRNG_5,
+    },{ .name = "PER_STRNG_6",  .addr = A_PER_STRNG_6,
+    },{ .name = "PER_STRNG_7",  .addr = A_PER_STRNG_7,
+    },{ .name = "PER_STRNG_8",  .addr = A_PER_STRNG_8,
+    },{ .name = "PER_STRNG_9",  .addr = A_PER_STRNG_9,
+    },{ .name = "PER_STRNG_10",  .addr = A_PER_STRNG_10,
+    },{ .name = "PER_STRNG_11",  .addr = A_PER_STRNG_11,
+    },{ .name = "CORE_OUTPUT",  .addr = A_CORE_OUTPUT,
+        .ro = 0xffffffff,
+        .post_read = trng_core_out_postr,
+    },{ .name = "RESET",  .addr = A_RESET,
+        .reset = 0x1,
+        .pre_write = trng_reset_prew,
+    },{ .name = "OSC_EN",  .addr = A_OSC_EN,
+    },{ .name = "TRNG_ISR",  .addr = A_TRNG_ISR,
+        .w1c = 0x3,
+        .post_write = trng_isr_postw,
+    },{ .name = "TRNG_IMR",  .addr = A_TRNG_IMR,
+        .reset = 0x3,
+        .ro = 0x3,
+    },{ .name = "TRNG_IER",  .addr = A_TRNG_IER,
+        .pre_write = trng_ier_prew,
+    },{ .name = "TRNG_IDR",  .addr = A_TRNG_IDR,
+        .pre_write = trng_idr_prew,
+    },{ .name = "SLV_ERR_CTRL",  .addr = A_SLV_ERR_CTRL,
+    }
+};
+
+static const MemoryRegionOps trng_ops = {
+    .read = trng_register_read,
+    .write = trng_register_write,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+    .valid = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
+};
+
+static void trng_init(Object *obj)
+{
+    XlnxVersalTRng *s = XLNX_VERSAL_TRNG(obj);
+    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
+    RegisterInfoArray *reg_array;
+
+    reg_array =
+        register_init_block32(DEVICE(obj), trng_regs_info,
+                              ARRAY_SIZE(trng_regs_info),
+                              s->regs_info, s->regs,
+                              &trng_ops,
+                              XLNX_VERSAL_TRNG_ERR_DEBUG,
+                              R_MAX * 4);
+
+    sysbus_init_mmio(sbd, &reg_array->mem);
+    sysbus_init_irq(sbd, &s->irq);
+
+    s->prng = g_rand_new();
+}
+
+static void trng_unrealize(DeviceState *dev)
+{
+    XlnxVersalTRng *s = XLNX_VERSAL_TRNG(dev);
+
+    g_rand_free(s->prng);
+    s->prng = NULL;
+}
+
+static void trng_reset_hold(Object *obj)
+{
+    trng_reset(XLNX_VERSAL_TRNG(obj));
+}
+
+static void trng_prop_fault_event_set(Object *obj, Visitor *v,
+                                      const char *name, void *opaque,
+                                      Error **errp)
+{
+    Property *prop = opaque;
+    uint32_t *events = object_field_prop_ptr(obj, prop);
+
+    visit_type_uint32(v, name, events, errp);
+    if (*errp) {
+        return;
+    }
+
+    trng_fault_event_set(XLNX_VERSAL_TRNG(obj), *events);
+}
+
+static const PropertyInfo trng_prop_fault_events = {
+    .name = "uint32:bits",
+    .description = "Set to trigger TRNG fault events",
+    .set = trng_prop_fault_event_set,
+    .realized_set_allowed = true,
+};
+
+static PropertyInfo trng_prop_uint64; /* to extend qdev_prop_uint64 */
+
+static Property trng_props[] = {
+    DEFINE_PROP_UINT64("forced-prng", XlnxVersalTRng, forced_prng_seed, 0),
+    DEFINE_PROP_UINT32("hw-version", XlnxVersalTRng, hw_version, 0x0200),
+    DEFINE_PROP("fips-fault-events", XlnxVersalTRng, forced_faults,
+                trng_prop_fault_events, uint32_t),
+
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static const VMStateDescription vmstate_trng = {
+    .name = TYPE_XLNX_VERSAL_TRNG,
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32(rand_count, XlnxVersalTRng),
+        VMSTATE_UINT64(rand_reseed, XlnxVersalTRng),
+        VMSTATE_UINT64(forced_prng_count, XlnxVersalTRng),
+        VMSTATE_UINT64_ARRAY(tst_seed, XlnxVersalTRng, 2),
+        VMSTATE_UINT32_ARRAY(regs, XlnxVersalTRng, R_MAX),
+        VMSTATE_END_OF_LIST(),
+    }
+};
+
+static void trng_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    ResettableClass *rc = RESETTABLE_CLASS(klass);
+
+    dc->vmsd = &vmstate_trng;
+    dc->unrealize = trng_unrealize;
+    rc->phases.hold = trng_reset_hold;
+
+    /* Clone uint64 property with set allowed after realized */
+    trng_prop_uint64 = qdev_prop_uint64;
+    trng_prop_uint64.realized_set_allowed = true;
+    trng_props[0].info = &trng_prop_uint64;
+
+    device_class_set_props(dc, trng_props);
+}
+
+static const TypeInfo trng_info = {
+    .name          = TYPE_XLNX_VERSAL_TRNG,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(XlnxVersalTRng),
+    .class_init    = trng_class_init,
+    .instance_init = trng_init,
+};
+
+static void trng_register_types(void)
+{
+    type_register_static(&trng_info);
+}
+
+type_init(trng_register_types)
diff --git a/include/hw/misc/xlnx-versal-trng.h b/include/hw/misc/xlnx-versal-trng.h
new file mode 100644
index 0000000000..0bcef8a613
--- /dev/null
+++ b/include/hw/misc/xlnx-versal-trng.h
@@ -0,0 +1,58 @@
+/*
+ * Non-crypto strength model of the True Random Number Generator
+ * in the AMD/Xilinx Versal device family.
+ *
+ * Copyright (c) 2017-2020 Xilinx Inc.
+ * Copyright (c) 2023 Advanced Micro Devices, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+#ifndef XLNX_VERSAL_TRNG_H
+#define XLNX_VERSAL_TRNG_H
+
+#include "hw/irq.h"
+#include "hw/sysbus.h"
+#include "hw/register.h"
+
+#define TYPE_XLNX_VERSAL_TRNG "xlnx.versal-trng"
+OBJECT_DECLARE_SIMPLE_TYPE(XlnxVersalTRng, XLNX_VERSAL_TRNG);
+
+#define RMAX_XLNX_VERSAL_TRNG ((0xf0 / 4) + 1)
+
+typedef struct XlnxVersalTRng {
+    SysBusDevice parent_obj;
+    qemu_irq irq;
+    GRand *prng;
+
+    uint32_t hw_version;
+    uint32_t forced_faults;
+
+    uint32_t rand_count;
+    uint64_t rand_reseed;
+
+    uint64_t forced_prng_seed;
+    uint64_t forced_prng_count;
+    uint64_t tst_seed[2];
+
+    uint32_t regs[RMAX_XLNX_VERSAL_TRNG];
+    RegisterInfo regs_info[RMAX_XLNX_VERSAL_TRNG];
+} XlnxVersalTRng;
+
+#undef RMAX_XLNX_VERSAL_TRNG
+#endif
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v4 2/3] hw/arm: xlnx-versal-virt: Add AMD/Xilinx TRNG device
  2023-10-17 19:32 [PATCH v4 0/3] AMD/Xilinx Versal TRNG support Tong Ho
  2023-10-17 19:32 ` [PATCH v4 1/3] hw/misc: Introduce AMD/Xilix Versal TRNG device Tong Ho
@ 2023-10-17 19:32 ` Tong Ho
  2023-10-27 13:03   ` Peter Maydell
  2023-10-17 19:32 ` [PATCH v4 3/3] tests/qtest: Introduce tests for AMD/Xilinx Versal " Tong Ho
  2 siblings, 1 reply; 10+ messages in thread
From: Tong Ho @ 2023-10-17 19:32 UTC (permalink / raw)
  To: qemu-arm
  Cc: qemu-devel, alistair, edgar.iglesias, peter.maydell,
	richard.henderson, frasse.iglesias, tong.ho

Connect the support for Versal True Random Number Generator
(TRNG) device.

Warning: unlike the TRNG component in a real device from the
Versal device familiy, the connected TRNG model is not of
cryptographic grade and is not intended for use cases when
cryptograpically strong TRNG is needed.

Signed-off-by: Tong Ho <tong.ho@amd.com>
---
 hw/arm/Kconfig               |  1 +
 hw/arm/xlnx-versal.c         | 16 ++++++++++++++++
 include/hw/arm/xlnx-versal.h |  5 +++++
 3 files changed, 22 insertions(+)

diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 7e68348440..0a3ff6748d 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -482,6 +482,7 @@ config XLNX_VERSAL
     select XLNX_BBRAM
     select XLNX_EFUSE_VERSAL
     select XLNX_USB_SUBSYS
+    select XLNX_VERSAL_TRNG
 
 config NPCM7XX
     bool
diff --git a/hw/arm/xlnx-versal.c b/hw/arm/xlnx-versal.c
index fa556d8764..4f74a64a0d 100644
--- a/hw/arm/xlnx-versal.c
+++ b/hw/arm/xlnx-versal.c
@@ -373,6 +373,21 @@ static void versal_create_rtc(Versal *s, qemu_irq *pic)
                        qdev_get_gpio_in(DEVICE(&s->pmc.apb_irq_orgate), 0));
 }
 
+static void versal_create_trng(Versal *s, qemu_irq *pic)
+{
+    SysBusDevice *sbd;
+    MemoryRegion *mr;
+
+    object_initialize_child(OBJECT(s), "trng", &s->pmc.trng,
+                            TYPE_XLNX_VERSAL_TRNG);
+    sbd = SYS_BUS_DEVICE(&s->pmc.trng);
+    sysbus_realize(sbd, &error_fatal);
+
+    mr = sysbus_mmio_get_region(sbd, 0);
+    memory_region_add_subregion(&s->mr_ps, MM_PMC_TRNG, mr);
+    sysbus_connect_irq(sbd, 0, pic[VERSAL_TRNG_IRQ]);
+}
+
 static void versal_create_xrams(Versal *s, qemu_irq *pic)
 {
     int nr_xrams = ARRAY_SIZE(s->lpd.xram.ctrl);
@@ -909,6 +924,7 @@ static void versal_realize(DeviceState *dev, Error **errp)
     versal_create_sds(s, pic);
     versal_create_pmc_apb_irq_orgate(s, pic);
     versal_create_rtc(s, pic);
+    versal_create_trng(s, pic);
     versal_create_xrams(s, pic);
     versal_create_bbram(s, pic);
     versal_create_efuse(s, pic);
diff --git a/include/hw/arm/xlnx-versal.h b/include/hw/arm/xlnx-versal.h
index 7b419f88c2..54f4b98d9d 100644
--- a/include/hw/arm/xlnx-versal.h
+++ b/include/hw/arm/xlnx-versal.h
@@ -31,6 +31,7 @@
 #include "hw/dma/xlnx_csu_dma.h"
 #include "hw/misc/xlnx-versal-crl.h"
 #include "hw/misc/xlnx-versal-pmc-iou-slcr.h"
+#include "hw/misc/xlnx-versal-trng.h"
 #include "hw/net/xlnx-versal-canfd.h"
 #include "hw/misc/xlnx-versal-cfu.h"
 #include "hw/misc/xlnx-versal-cframe-reg.h"
@@ -116,6 +117,7 @@ struct Versal {
         } iou;
 
         XlnxZynqMPRTC rtc;
+        XlnxVersalTRng trng;
         XlnxBBRam bbram;
         XlnxEFuse efuse;
         XlnxVersalEFuseCtrl efuse_ctrl;
@@ -160,6 +162,7 @@ struct Versal {
 #define VERSAL_OSPI_IRQ            124
 #define VERSAL_SD0_IRQ_0           126
 #define VERSAL_EFUSE_IRQ           139
+#define VERSAL_TRNG_IRQ            141
 #define VERSAL_RTC_ALARM_IRQ       142
 #define VERSAL_RTC_SECONDS_IRQ     143
 
@@ -329,4 +332,6 @@ struct Versal {
 #define MM_PMC_CRP_SIZE             0x10000
 #define MM_PMC_RTC                  0xf12a0000
 #define MM_PMC_RTC_SIZE             0x10000
+#define MM_PMC_TRNG                 0xf1230000
+#define MM_PMC_TRNG_SIZE            0x10000
 #endif
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v4 3/3] tests/qtest: Introduce tests for AMD/Xilinx Versal TRNG device
  2023-10-17 19:32 [PATCH v4 0/3] AMD/Xilinx Versal TRNG support Tong Ho
  2023-10-17 19:32 ` [PATCH v4 1/3] hw/misc: Introduce AMD/Xilix Versal TRNG device Tong Ho
  2023-10-17 19:32 ` [PATCH v4 2/3] hw/arm: xlnx-versal-virt: Add AMD/Xilinx " Tong Ho
@ 2023-10-17 19:32 ` Tong Ho
  2023-10-27 13:03   ` Peter Maydell
  2023-10-27 13:15   ` Peter Maydell
  2 siblings, 2 replies; 10+ messages in thread
From: Tong Ho @ 2023-10-17 19:32 UTC (permalink / raw)
  To: qemu-arm
  Cc: qemu-devel, alistair, edgar.iglesias, peter.maydell,
	richard.henderson, frasse.iglesias, tong.ho

Signed-off-by: Tong Ho <tong.ho@amd.com>
---
 tests/qtest/meson.build             |   2 +-
 tests/qtest/xlnx-versal-trng-test.c | 486 ++++++++++++++++++++++++++++
 2 files changed, 487 insertions(+), 1 deletion(-)
 create mode 100644 tests/qtest/xlnx-versal-trng-test.c

diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 66795cfcd2..593ca6714b 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -216,7 +216,7 @@ qtests_aarch64 = \
   (config_all.has_key('CONFIG_TCG') and config_all_devices.has_key('CONFIG_TPM_TIS_SYSBUS') ?            \
     ['tpm-tis-device-test', 'tpm-tis-device-swtpm-test'] : []) +                                         \
   (config_all_devices.has_key('CONFIG_XLNX_ZYNQMP_ARM') ? ['xlnx-can-test', 'fuzz-xlnx-dp-test'] : []) + \
-  (config_all_devices.has_key('CONFIG_XLNX_VERSAL') ? ['xlnx-canfd-test'] : []) + \
+  (config_all_devices.has_key('CONFIG_XLNX_VERSAL') ? ['xlnx-canfd-test', 'xlnx-versal-trng-test'] : []) + \
   (config_all_devices.has_key('CONFIG_RASPI') ? ['bcm2835-dma-test'] : []) +  \
   (config_all.has_key('CONFIG_TCG') and                                            \
    config_all_devices.has_key('CONFIG_TPM_TIS_I2C') ? ['tpm-tis-i2c-test'] : []) + \
diff --git a/tests/qtest/xlnx-versal-trng-test.c b/tests/qtest/xlnx-versal-trng-test.c
new file mode 100644
index 0000000000..dc19c1e83b
--- /dev/null
+++ b/tests/qtest/xlnx-versal-trng-test.c
@@ -0,0 +1,486 @@
+/*
+ * QTests for the Xilinx Versal True Random Number Generator device
+ *
+ * Copyright (c) 2023 Advanced Micro Devices, Inc.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "libqtest-single.h"
+
+/* Base Address */
+#define TRNG_BASEADDR      (0xf1230000)
+
+/* TRNG_INT_CTRL */
+#define R_TRNG_INT_CTRL                 (0x0000)
+#define   TRNG_INT_CTRL_CERTF_RST_MASK  (1 << 5)
+#define   TRNG_INT_CTRL_DTF_RST_MASK    (1 << 4)
+#define   TRNG_INT_CTRL_DONE_RST_MASK   (1 << 3)
+#define   TRNG_INT_CTRL_CERTF_EN_MASK   (1 << 2)
+#define   TRNG_INT_CTRL_DTF_EN_MASK     (1 << 1)
+#define   TRNG_INT_CTRL_DONE_EN_MASK    (1)
+
+/* TRNG_STATUS */
+#define R_TRNG_STATUS              (0x0004)
+#define   TRNG_STATUS_QCNT_SHIFT   (9)
+#define   TRNG_STATUS_QCNT_MASK    (7 << TRNG_STATUS_QCNT_SHIFT)
+#define   TRNG_STATUS_CERTF_MASK   (1 << 3)
+#define   TRNG_STATUS_DTF_MASK     (1 << 1)
+#define   TRNG_STATUS_DONE_MASK    (1)
+
+/* TRNG_CTRL */
+#define R_TRNG_CTRL                (0x0008)
+#define   TRNG_CTRL_PERSODISABLE_MASK   (1 << 10)
+#define   TRNG_CTRL_SINGLEGENMODE_MASK  (1 << 9)
+#define   TRNG_CTRL_PRNGMODE_MASK       (1 << 7)
+#define   TRNG_CTRL_TSTMODE_MASK        (1 << 6)
+#define   TRNG_CTRL_PRNGSTART_MASK      (1 << 5)
+#define   TRNG_CTRL_PRNGXS_MASK         (1 << 3)
+#define   TRNG_CTRL_TRSSEN_MASK         (1 << 2)
+#define   TRNG_CTRL_QERTUEN_MASK        (1 << 1)
+#define   TRNG_CTRL_PRNGSRST_MASK       (1)
+
+/* TRNG_EXT_SEED_0 ... _11 */
+#define R_TRNG_EXT_SEED_0          (0x0040)
+#define R_TRNG_EXT_SEED_11         (R_TRNG_EXT_SEED_0 + 4 * 11)
+
+/* TRNG_PER_STRNG_0 ... 11 */
+#define R_TRNG_PER_STRNG_0         (0x0080)
+#define R_TRNG_PER_STRNG_11        (R_TRNG_PER_STRNG_0 + 4 * 11)
+
+/* TRNG_CORE_OUTPUT */
+#define R_TRNG_CORE_OUTPUT         (0x00c0)
+
+/* TRNG_RESET */
+#define R_TRNG_RESET               (0x00d0)
+#define   TRNG_RESET_VAL_MASK      (1)
+
+/* TRNG_OSC_EN */
+#define R_TRNG_OSC_EN              (0x00d4)
+#define   TRNG_OSC_EN_VAL_MASK     (1)
+
+/* TRNG_TRNG_ISR, _IMR, _IER, _IDR */
+#define R_TRNG_ISR                 (0x00e0)
+#define R_TRNG_IMR                 (0x00e4)
+#define R_TRNG_IER                 (0x00e8)
+#define R_TRNG_IDR                 (0x00ec)
+#define   TRNG_IRQ_SLVERR_MASK     (1 << 1)
+#define   TRNG_IRQ_CORE_INT_MASK   (1)
+
+#define FAILED(FMT, ...) g_error("%s(): " FMT, __func__, ## __VA_ARGS__)
+
+static const uint32_t prng_seed[12] = {
+    0x01234567, 0x12345678, 0x23456789, 0x3456789a, 0x456789ab, 0x56789abc,
+    0x76543210, 0x87654321, 0x98765432, 0xa9876543, 0xba987654, 0xfedcba98,
+};
+
+static const uint32_t pers_str[12] = {
+    0x76543210, 0x87654321, 0x98765432, 0xa9876543, 0xba987654, 0xfedcba98,
+    0x01234567, 0x12345678, 0x23456789, 0x3456789a, 0x456789ab, 0x56789abc,
+};
+
+static void trng_test_start(void)
+{
+    qtest_start("-machine xlnx-versal-virt");
+}
+
+static void trng_test_stop(void)
+{
+    qtest_end();
+}
+
+static void trng_test_set_uint_prop(const char *name, uint64_t value)
+{
+    const char *path = "/machine/xlnx-versal/trng";
+    QDict *response;
+
+    response = qmp("{ 'execute': 'qom-set',"
+                    " 'arguments': {"
+                       " 'path': %s,"
+                       " 'property': %s,"
+                       " 'value': %llu"
+                      "} }", path,
+                   name, (unsigned long long)value);
+    g_assert(qdict_haskey(response, "return"));
+    qobject_unref(response);
+}
+
+static void trng_write(unsigned ra, uint32_t val)
+{
+    writel(TRNG_BASEADDR + ra, val);
+}
+
+static uint32_t trng_read(unsigned ra)
+{
+    return readl(TRNG_BASEADDR + ra);
+}
+
+static void trng_bit_set(unsigned ra, uint32_t bits)
+{
+    trng_write(ra, (trng_read(ra) | bits));
+}
+
+static void trng_bit_clr(unsigned ra, uint32_t bits)
+{
+    trng_write(ra, (trng_read(ra) & ~bits));
+}
+
+static void trng_ctrl_set(uint32_t bits)
+{
+    trng_bit_set(R_TRNG_CTRL, bits);
+}
+
+static void trng_ctrl_clr(uint32_t bits)
+{
+    trng_bit_clr(R_TRNG_CTRL, bits);
+}
+
+static uint32_t trng_status(void)
+{
+    return trng_read(R_TRNG_STATUS);
+}
+
+static unsigned trng_qcnt(void)
+{
+    uint32_t sta = trng_status();
+
+    return (sta & TRNG_STATUS_QCNT_MASK) >> TRNG_STATUS_QCNT_SHIFT;
+}
+
+static const char *trng_info(void)
+{
+    uint32_t sta = trng_status();
+    uint32_t ctl = trng_read(R_TRNG_CTRL);
+
+    static char info[64];
+
+    snprintf(info, sizeof(info), "; status=0x%x, ctrl=0x%x", sta, ctl);
+    return info;
+}
+
+static void trng_wait(uint32_t wait_mask, bool on, const char *act)
+{
+    time_t tmo = time(NULL) + 2; /* at most 2 seconds */
+    uint32_t event_mask = 0;
+    uint32_t clear_mask = 0;
+
+    /*
+     * Only selected bits are events in R_TRNG_STATUS, and
+     * clear them needs to go through R_INT_CTRL.
+     */
+    if (wait_mask & TRNG_STATUS_CERTF_MASK) {
+        event_mask |= TRNG_STATUS_CERTF_MASK;
+        clear_mask |= TRNG_INT_CTRL_CERTF_RST_MASK;
+    }
+    if (wait_mask & TRNG_STATUS_DTF_MASK) {
+        event_mask |= TRNG_STATUS_DTF_MASK;
+        clear_mask |= TRNG_INT_CTRL_DTF_RST_MASK;
+    }
+    if (wait_mask & TRNG_STATUS_DONE_MASK) {
+        event_mask |= TRNG_STATUS_DONE_MASK;
+        clear_mask |= TRNG_INT_CTRL_DONE_RST_MASK;
+    }
+
+    for (;;) {
+        bool sta = !!(trng_status() & event_mask);
+
+        if ((on ^ sta) == 0) {
+            break;
+        }
+
+        if (time(NULL) >= tmo) {
+            FAILED("%s: Timed out waiting for event 0x%x to be %d%s",
+                   act, event_mask, (int)on, trng_info());
+        }
+
+        g_usleep(10000);
+    }
+
+    /* Remove event */
+    trng_bit_set(R_TRNG_INT_CTRL, clear_mask);
+
+    if (!!(trng_read(R_TRNG_STATUS) & event_mask)) {
+        FAILED("%s: Event 0x%0x stuck at 1 after clear: %s",
+               act, event_mask, trng_info());
+    }
+}
+
+static void trng_wait_done(const char *act)
+{
+    trng_wait(TRNG_STATUS_DONE_MASK, true, act);
+}
+
+static void trng_wait_dtf(void)
+{
+    trng_wait(TRNG_STATUS_DTF_MASK, true, "DTF injection");
+}
+
+static void trng_wait_certf(void)
+{
+    trng_wait(TRNG_STATUS_CERTF_MASK, true, "CERTF injection");
+}
+
+static void trng_reset(void)
+{
+    trng_write(R_TRNG_RESET, TRNG_RESET_VAL_MASK);
+    trng_write(R_TRNG_RESET, 0);
+}
+
+static void trng_load(unsigned r0, const uint32_t *b384)
+{
+    static const uint32_t zero[12] = { 0 };
+    unsigned k;
+
+    if (!b384) {
+        b384 = zero;
+    }
+
+    for (k = 0; k < 12; k++) {
+        trng_write(r0 + 4 * k, b384[k]);
+    }
+}
+
+static void trng_reseed(const uint32_t *seed)
+{
+    const char *act;
+    uint32_t ctl;
+
+    ctl = TRNG_CTRL_PRNGSTART_MASK |
+          TRNG_CTRL_PRNGXS_MASK |
+          TRNG_CTRL_TRSSEN_MASK;
+
+    trng_ctrl_clr(ctl | TRNG_CTRL_PRNGMODE_MASK);
+
+    if (seed) {
+        trng_load(R_TRNG_EXT_SEED_0, seed);
+        act = "Reseed PRNG";
+        ctl &= ~TRNG_CTRL_TRSSEN_MASK;
+    } else {
+        trng_write(R_TRNG_OSC_EN, TRNG_OSC_EN_VAL_MASK);
+        act = "Reseed TRNG";
+        ctl &= ~TRNG_CTRL_PRNGXS_MASK;
+    }
+
+    trng_ctrl_set(ctl);
+    trng_wait_done(act);
+    trng_ctrl_clr(TRNG_CTRL_PRNGSTART_MASK);
+}
+
+static void trng_generate(bool auto_enb)
+{
+    uint32_t ctl;
+
+    ctl = TRNG_CTRL_PRNGSTART_MASK | TRNG_CTRL_SINGLEGENMODE_MASK;
+    trng_ctrl_clr(ctl);
+
+    if (auto_enb) {
+        ctl &= ~TRNG_CTRL_SINGLEGENMODE_MASK;
+    }
+
+    trng_ctrl_set(ctl | TRNG_CTRL_PRNGMODE_MASK);
+
+    trng_wait_done("Generate");
+    g_assert(trng_qcnt() != 7);
+}
+
+static size_t trng_collect(uint32_t *rnd, size_t cnt)
+{
+    size_t i;
+
+    for (i = 0; i < cnt; i++) {
+        if (trng_qcnt() == 0) {
+            return i;
+        }
+
+        rnd[i] = trng_read(R_TRNG_CORE_OUTPUT);
+    }
+
+    return i;
+}
+
+static void trng_test_autogen(void)
+{
+    const size_t cnt = 512 / 32;
+    uint32_t rng[cnt], prng[cnt];
+    size_t n;
+
+    trng_reset();
+
+    /* PRNG run #1 */
+    trng_reseed(prng_seed);
+    trng_generate(true);
+
+    n = trng_collect(prng, cnt);
+    if (n != cnt) {
+        FAILED("PRNG_1 Auto-gen test failed: expected = %u, got = %u",
+               (unsigned)cnt, (unsigned)n);
+    }
+
+    /* TRNG, should not match PRNG */
+    trng_reseed(NULL);
+    trng_generate(true);
+
+    n = trng_collect(rng, cnt);
+    if (n != cnt) {
+        FAILED("TRNG Auto-gen test failed: expected = %u, got = %u",
+               (unsigned)cnt, (unsigned)n);
+    }
+
+    /* PRNG #2: should matches run #1 */
+    trng_reseed(prng_seed);
+    trng_generate(true);
+
+    n = trng_collect(rng, cnt);
+    if (n != cnt) {
+        FAILED("PRNG_2 Auto-gen test failed: expected = %u, got = %u",
+               (unsigned)cnt, (unsigned)n);
+    }
+
+    if (memcmp(rng, prng, sizeof(rng))) {
+        FAILED("PRNG_2 Auto-gen test failed: does not match PRNG_1");
+    }
+}
+
+static void trng_test_oneshot(void)
+{
+    const size_t cnt = 512 / 32;
+    uint32_t rng[cnt];
+    size_t n;
+
+    trng_reset();
+
+    /* PRNG run #1 */
+    trng_reseed(prng_seed);
+    trng_generate(false);
+
+    n = trng_collect(rng, cnt);
+    if (n == cnt) {
+        FAILED("PRNG_1 One-shot gen test failed");
+    }
+
+    /* TRNG, should not match PRNG */
+    trng_reseed(NULL);
+    trng_generate(false);
+
+    n = trng_collect(rng, cnt);
+    if (n == cnt) {
+        FAILED("TRNG One-shot test failed");
+    }
+}
+
+static void trng_test_per_str(void)
+{
+    const size_t cnt = 512 / 32;
+    uint32_t rng[cnt], prng[cnt];
+    size_t n;
+
+    trng_reset();
+
+    /* #1: disabled */
+    trng_ctrl_set(TRNG_CTRL_PERSODISABLE_MASK);
+    trng_reseed(prng_seed);
+    trng_ctrl_clr(TRNG_CTRL_PERSODISABLE_MASK);
+
+    trng_generate(true);
+    n = trng_collect(prng, cnt);
+    g_assert_cmpuint(n, ==, cnt);
+
+    /* #2: zero string should match personalization disabled */
+    trng_load(R_TRNG_PER_STRNG_0, NULL);
+    trng_reseed(prng_seed);
+
+    trng_generate(true);
+    n = trng_collect(rng, cnt);
+    g_assert_cmpuint(n, ==, cnt);
+
+    if (memcmp(rng, prng, sizeof(rng))) {
+        FAILED("Failed: PER_DISABLE != PER_STRNG_ALL_ZERO");
+    }
+
+    /* #3: non-zero string should not match personalization disabled */
+    trng_load(R_TRNG_PER_STRNG_0, pers_str);
+    trng_reseed(prng_seed);
+
+    trng_generate(true);
+    n = trng_collect(rng, cnt);
+    g_assert_cmpuint(n, ==, cnt);
+
+    if (!memcmp(rng, prng, sizeof(rng))) {
+        FAILED("Failed: PER_DISABLE == PER_STRNG_NON_ZERO");
+    }
+}
+
+static void trng_test_forced_prng(void)
+{
+    const char *prop = "forced-prng";
+    const uint64_t seed = 0xdeadbeefbad1bad0ULL;
+
+    const size_t cnt = 512 / 32;
+    uint32_t rng[cnt], prng[cnt];
+    size_t n;
+
+    trng_reset();
+    trng_test_set_uint_prop(prop, seed);
+
+    /* TRNG run #1 */
+    trng_reset();
+    trng_reseed(NULL);
+    trng_generate(true);
+
+    n = trng_collect(prng, cnt);
+    g_assert_cmpuint(n, ==, cnt);
+
+    /* TRNG run #2 should match run #1 */
+    trng_reset();
+    trng_reseed(NULL);
+    trng_generate(true);
+
+    n = trng_collect(rng, cnt);
+    g_assert_cmpuint(n, ==, cnt);
+
+    if (memcmp(rng, prng, sizeof(rng))) {
+        FAILED("Forced-prng test failed: results do not match");
+    }
+}
+
+static void trng_test_fault_events(void)
+{
+    const char *prop = "fips-fault-events";
+
+    trng_reset();
+
+    /* Fault events only when TRSS is enabled */
+    trng_write(R_TRNG_OSC_EN, TRNG_OSC_EN_VAL_MASK);
+    trng_ctrl_set(TRNG_CTRL_TRSSEN_MASK);
+
+    trng_test_set_uint_prop(prop, TRNG_STATUS_CERTF_MASK);
+    trng_wait_certf();
+
+    trng_test_set_uint_prop(prop, TRNG_STATUS_DTF_MASK);
+    trng_wait_dtf();
+
+    trng_reset();
+}
+
+int main(int argc, char **argv)
+{
+    int rc;
+
+    g_test_init(&argc, &argv, NULL);
+
+    #define TRNG_TEST_ADD(n) \
+            qtest_add_func("/hw/misc/xlnx-versal-trng/" #n, trng_test_ ## n);
+    TRNG_TEST_ADD(autogen);
+    TRNG_TEST_ADD(oneshot);
+    TRNG_TEST_ADD(per_str);
+    TRNG_TEST_ADD(forced_prng);
+    TRNG_TEST_ADD(fault_events);
+    #undef TRNG_TEST_ADD
+
+    trng_test_start();
+    rc = g_test_run();
+    trng_test_stop();
+
+    return rc;
+}
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 1/3] hw/misc: Introduce AMD/Xilix Versal TRNG device
  2023-10-17 19:32 ` [PATCH v4 1/3] hw/misc: Introduce AMD/Xilix Versal TRNG device Tong Ho
@ 2023-10-27 12:54   ` Peter Maydell
  0 siblings, 0 replies; 10+ messages in thread
From: Peter Maydell @ 2023-10-27 12:54 UTC (permalink / raw)
  To: Tong Ho
  Cc: qemu-arm, qemu-devel, alistair, edgar.iglesias, richard.henderson,
	frasse.iglesias

On Tue, 17 Oct 2023 at 20:32, Tong Ho <tong.ho@amd.com> wrote:
>
> This adds a non-cryptographic grade implementation of the
> model for the True Random Number Generator (TRNG) component
> in AMD/Xilinx Versal device family.
>
> This implements all 3 modes defined by the actual hardware
> specs, all of which selectable by guest software at will
> at anytime:
> 1) PRNG mode, in which the generated sequence is required to
>    be reproducible after reseeded by the same 384-bit value
>    as supplied by guest software.
> 2) Test mode, in which the generated sequence is required to
>    be reproducible ater reseeded by the same 128-bit test
>    seed supplied by guest software.
> 3) TRNG mode, in which non-reproducible sequence is generated
>    based on periodic reseed by a suitable entropy source.
>
> This model is only intended for non-real world testing of
> guest software, where cryptographically strong PRNG or TRNG
> is not needed.
>
> This model supports versions 1 & 2 of the device, with
> default to be version 2; the 'hw-version' uint32 property
> can be set to 0x0100 to override the default.
>
> Other implemented properties:
> - 'forced-prng', uint64
>   When set to non-zero, mode 3's entropy source is implemented
>   as a deterministic sequence based on the given value and other
>   deterministic parameters.
>   This option allows the emulation to test guest software using
>   mode 3 and to reproduce data-dependent defects.
>
> - 'fips-fault-events', uint32, bit-mask
>   bit 3: Triggers the SP800-90B entropy health test fault irq
>   bit 1: Triggers the FIPS 140-2 continuous test fault irq
>
> Signed-off-by: Tong Ho <tong.ho@amd.com>
> ---

> +static void trng_le128(uint32_t *le128, uint64_t h00, uint64_t h64)
> +{
> +    le128[0] = cpu_to_le32(h00 & 0xffffffffU);
> +    le128[1] = cpu_to_le32(h00 >> 32);
> +    le128[2] = cpu_to_le32(h64 & 0xffffffffU);
> +    le128[3] = cpu_to_le32(h64 >> 32);

Why are we using cpu_to_le32() here ? We have a host-order
pair of uint64_t values, and we want a set of host-order
uint32_t values (because we're passing a guint32 array
to the glib g_rand_set_seed_array() function). So I
think we want

   seedarray[0] = extract64(h00, 0, 32);
   seedarray[1] = extract64(h00, 32, 32);
   seedarray[2] = extract64(h64, 0, 32);
   seedarray[3] = extract64(h64, 32, 32);

(I renamed the function argument because le128 is a
red herring, and used extract64 because I find that clearer.
The function itself could also use a different name now it
isn't doing anything little-endian related.)

> +}
> +
> +static void trng_le384(uint32_t *le384, const uint32_t *h384)
> +{
> +    size_t i;
> +
> +    for (i = 0; i < (384 / 32); i++) {
> +        le384[i] = cpu_to_le32(h384[i]);
> +    }

Similarly here we have an array of host-order 32-bit values
(from the s->regs[] array) and we want an array of host-order
32-bit values for g_rand_set_seed_array(), so the cpu_to_le32()
is unnecessary (and we're just doing a copy).

> +}
> +
> +static void trng_reseed(XlnxVersalTRng *s)
> +{
> +    bool ext_seed = ARRAY_FIELD_EX32(s->regs, CTRL, PRNGXS);
> +    bool pers_disabled = ARRAY_FIELD_EX32(s->regs, CTRL, PERSODISABLE);
> +
> +    enum {
> +        U384_U32 = 384 / 32,
> +    };
> +
> +    /*
> +     * Maximum seed length is len(personalized string) + len(ext seed).
> +     *
> +     * Use little-endian to ensure guest sequence being indepedent of
> +     * host endian.
> +     */
> +    guint32 gs[U384_U32 * 2], *seed = &gs[U384_U32];
> +
> +    /*
> +     * A disabled personalized string is the same as
> +     * a string with all zeros.
> +     *
> +     * The device's hardware spec defines 3 modes (all selectable
> +     * by guest at will and at anytime):
> +     * 1) External seeding
> +     *    This is a PRNG mode, in which the produced sequence shall
> +     *    be reproducible if reseeded by the same 384-bit seed, as
> +     *    supplied by guest software.
> +     * 2) Test seeding
> +     *    This is a PRNG mode, in which the produced sequence shall
> +     *    be reproducible if reseeded by a 128-bit test seed, as
> +     *    supplied by guest software.
> +     * 3) Truly-random seeding
> +     *    This is the TRNG mode, in which the produced sequence is
> +     *    periodically reseeded by a crypto-strength entropy source.
> +     *
> +     * To assist debugging of certain classes of software defects,
> +     * this QEMU model implements a 4th mode,
> +     * 4) Forced PRNG
> +     *    When in this mode, a reproducible sequence is generated
> +     *    if software has selected the TRNG mode (mode 2).
> +     *
> +     *    This emulation-only mode can only be selected by setting
> +     *    the uint64 property 'forced-prng' to a non-zero value.
> +     *    Guest software cannot select this mode.
> +     */
> +    memset(gs, 0, sizeof(gs));
> +    stb_p((uint8_t *)gs + sizeof(gs) - 1, 1);

This looks like it gives different results on big and little
endian. The gs[] array is an array of g_uint32, not an
array of bytes. What's the intention behind setting this
byte to 1, anyway?

> +
> +    if (!pers_disabled) {
> +        trng_le384(gs, &s->regs[R_PER_STRNG_0]);
> +    }
> +
> +    if (ext_seed) {
> +        trng_le384(seed, &s->regs[R_EXT_SEED_0]);
> +    } else if (trng_test_enabled(s)) {
> +        trng_le128(seed, s->tst_seed[0], s->tst_seed[1]);
> +    } else if (s->forced_prng_seed) {
> +        s->forced_prng_count++;
> +        trng_le128(seed, s->forced_prng_count, s->forced_prng_seed);
> +    } else {
> +        qemu_guest_getrandom_nofail(seed, U384_U32 * 4);
> +    }
> +
> +    g_rand_set_seed_array(s->prng, gs, ARRAY_SIZE(gs));
> +
> +    s->rand_count = 0;
> +    s->rand_reseed = 1ULL << 48;
> +}

thanks
-- PMM


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 3/3] tests/qtest: Introduce tests for AMD/Xilinx Versal TRNG device
  2023-10-17 19:32 ` [PATCH v4 3/3] tests/qtest: Introduce tests for AMD/Xilinx Versal " Tong Ho
@ 2023-10-27 13:03   ` Peter Maydell
  2023-10-30  6:25     ` Ho, Tong
  2023-10-27 13:15   ` Peter Maydell
  1 sibling, 1 reply; 10+ messages in thread
From: Peter Maydell @ 2023-10-27 13:03 UTC (permalink / raw)
  To: Tong Ho
  Cc: qemu-arm, qemu-devel, alistair, edgar.iglesias, richard.henderson,
	frasse.iglesias

On Tue, 17 Oct 2023 at 20:32, Tong Ho <tong.ho@amd.com> wrote:
>
> Signed-off-by: Tong Ho <tong.ho@amd.com>
> ---
>  tests/qtest/meson.build             |   2 +-
>  tests/qtest/xlnx-versal-trng-test.c | 486 ++++++++++++++++++++++++++++
>  2 files changed, 487 insertions(+), 1 deletion(-)
>  create mode 100644 tests/qtest/xlnx-versal-trng-test.c


> +static void trng_wait(uint32_t wait_mask, bool on, const char *act)
> +{
> +    time_t tmo = time(NULL) + 2; /* at most 2 seconds */

Don't put timeouts this short into tests, please; ideally,
avoid them entirely. Sometimes the CI machines are heavily
loaded and the test might not be able to complete in a
short time like this. If you must have a timeout, it should
be at least a minute. (And see below on whether we need one.)

> +    uint32_t event_mask = 0;
> +    uint32_t clear_mask = 0;
> +
> +    /*
> +     * Only selected bits are events in R_TRNG_STATUS, and
> +     * clear them needs to go through R_INT_CTRL.
> +     */
> +    if (wait_mask & TRNG_STATUS_CERTF_MASK) {
> +        event_mask |= TRNG_STATUS_CERTF_MASK;
> +        clear_mask |= TRNG_INT_CTRL_CERTF_RST_MASK;
> +    }
> +    if (wait_mask & TRNG_STATUS_DTF_MASK) {
> +        event_mask |= TRNG_STATUS_DTF_MASK;
> +        clear_mask |= TRNG_INT_CTRL_DTF_RST_MASK;
> +    }
> +    if (wait_mask & TRNG_STATUS_DONE_MASK) {
> +        event_mask |= TRNG_STATUS_DONE_MASK;
> +        clear_mask |= TRNG_INT_CTRL_DONE_RST_MASK;
> +    }
> +
> +    for (;;) {
> +        bool sta = !!(trng_status() & event_mask);
> +
> +        if ((on ^ sta) == 0) {
> +            break;
> +        }
> +
> +        if (time(NULL) >= tmo) {
> +            FAILED("%s: Timed out waiting for event 0x%x to be %d%s",
> +                   act, event_mask, (int)on, trng_info());
> +        }
> +
> +        g_usleep(10000);

Why does this test need to use sleeps and timeouts?
A qtest test controls the guest 'clock' directly, so
usually they're completely deterministic. Is there some
behaviour in the TRNG device which is asynchronous (in
a way not driven from the QEMU guest clock) that I've missed ?

> +    }
> +
> +    /* Remove event */
> +    trng_bit_set(R_TRNG_INT_CTRL, clear_mask);
> +
> +    if (!!(trng_read(R_TRNG_STATUS) & event_mask)) {
> +        FAILED("%s: Event 0x%0x stuck at 1 after clear: %s",
> +               act, event_mask, trng_info());
> +    }
> +}

thanks
-- PMM


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 2/3] hw/arm: xlnx-versal-virt: Add AMD/Xilinx TRNG device
  2023-10-17 19:32 ` [PATCH v4 2/3] hw/arm: xlnx-versal-virt: Add AMD/Xilinx " Tong Ho
@ 2023-10-27 13:03   ` Peter Maydell
  0 siblings, 0 replies; 10+ messages in thread
From: Peter Maydell @ 2023-10-27 13:03 UTC (permalink / raw)
  To: Tong Ho
  Cc: qemu-arm, qemu-devel, alistair, edgar.iglesias, richard.henderson,
	frasse.iglesias

On Tue, 17 Oct 2023 at 20:32, Tong Ho <tong.ho@amd.com> wrote:
>
> Connect the support for Versal True Random Number Generator
> (TRNG) device.
>
> Warning: unlike the TRNG component in a real device from the
> Versal device familiy, the connected TRNG model is not of
> cryptographic grade and is not intended for use cases when
> cryptograpically strong TRNG is needed.
>
> Signed-off-by: Tong Ho <tong.ho@amd.com>
> ---

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

thanks
-- PMM


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 3/3] tests/qtest: Introduce tests for AMD/Xilinx Versal TRNG device
  2023-10-17 19:32 ` [PATCH v4 3/3] tests/qtest: Introduce tests for AMD/Xilinx Versal " Tong Ho
  2023-10-27 13:03   ` Peter Maydell
@ 2023-10-27 13:15   ` Peter Maydell
  1 sibling, 0 replies; 10+ messages in thread
From: Peter Maydell @ 2023-10-27 13:15 UTC (permalink / raw)
  To: Tong Ho
  Cc: qemu-arm, qemu-devel, alistair, edgar.iglesias, richard.henderson,
	frasse.iglesias

On Tue, 17 Oct 2023 at 20:32, Tong Ho <tong.ho@amd.com> wrote:
>
> Signed-off-by: Tong Ho <tong.ho@amd.com>
> ---
>  tests/qtest/meson.build             |   2 +-
>  tests/qtest/xlnx-versal-trng-test.c | 486 ++++++++++++++++++++++++++++
>  2 files changed, 487 insertions(+), 1 deletion(-)
>  create mode 100644 tests/qtest/xlnx-versal-trng-test.c
>
> diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
> index 66795cfcd2..593ca6714b 100644
> --- a/tests/qtest/meson.build
> +++ b/tests/qtest/meson.build
> @@ -216,7 +216,7 @@ qtests_aarch64 = \
>    (config_all.has_key('CONFIG_TCG') and config_all_devices.has_key('CONFIG_TPM_TIS_SYSBUS') ?            \
>      ['tpm-tis-device-test', 'tpm-tis-device-swtpm-test'] : []) +                                         \
>    (config_all_devices.has_key('CONFIG_XLNX_ZYNQMP_ARM') ? ['xlnx-can-test', 'fuzz-xlnx-dp-test'] : []) + \
> -  (config_all_devices.has_key('CONFIG_XLNX_VERSAL') ? ['xlnx-canfd-test'] : []) + \
> +  (config_all_devices.has_key('CONFIG_XLNX_VERSAL') ? ['xlnx-canfd-test', 'xlnx-versal-trng-test'] : []) + \
>    (config_all_devices.has_key('CONFIG_RASPI') ? ['bcm2835-dma-test'] : []) +  \
>    (config_all.has_key('CONFIG_TCG') and                                            \
>     config_all_devices.has_key('CONFIG_TPM_TIS_I2C') ? ['tpm-tis-i2c-test'] : []) + \
> diff --git a/tests/qtest/xlnx-versal-trng-test.c b/tests/qtest/xlnx-versal-trng-test.c
> new file mode 100644
> index 0000000000..dc19c1e83b
> --- /dev/null
> +++ b/tests/qtest/xlnx-versal-trng-test.c
> @@ -0,0 +1,486 @@
> +/*
> + * QTests for the Xilinx Versal True Random Number Generator device
> + *
> + * Copyright (c) 2023 Advanced Micro Devices, Inc.
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +

> +#define FAILED(FMT, ...) g_error("%s(): " FMT, __func__, ## __VA_ARGS__)

This fails to build for Windows because this macro name clashes
with one from the system winerror.h:

../tests/qtest/xlnx-versal-trng-test.c:71: error: "FAILED" redefined [-Werror]
71 | #define FAILED(FMT, ...) g_error("%s(): " FMT, __func__, ## __VA_ARGS__)

Also, qtests should not be using g_error() to report errors, because
this turns into a call to abort(), which the test harness will
not report nicely as an error. Use g_assert_true() or one of
the family of g_assert_cmp*() functions to test checks that
you want to be test passes or failures (but *not* g_assert()).

thanks
-- PMM


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 3/3] tests/qtest: Introduce tests for AMD/Xilinx Versal TRNG device
  2023-10-27 13:03   ` Peter Maydell
@ 2023-10-30  6:25     ` Ho, Tong
  2023-10-30  9:57       ` Peter Maydell
  0 siblings, 1 reply; 10+ messages in thread
From: Ho, Tong @ 2023-10-30  6:25 UTC (permalink / raw)
  To: Peter Maydell
  Cc: qemu-arm@nongnu.org, qemu-devel@nongnu.org,
	alistair@alistair23.me, edgar.iglesias@gmail.com,
	richard.henderson@linaro.org, frasse.iglesias@gmail.com

[-- Attachment #1: Type: text/plain, Size: 3887 bytes --]

Hi Peter,

This is in regard to your comment on the use of g_usleep() in waiting for
an event-bit update from the device under test.

The TRNG device model presented in patch #1 does not have any asynchronous behavior.

So, do you mean that, although the qtest process and the qemu-system-* process
are running as 2 separate processes, qtest infrastructure already has the necessary
mechanism to ensure that:

1. After qtest test sets a register that has the correct deterministic behavior to update an event bit,

2. The same qtest test subsequently issuing a register read will be guaranteed to observe the updated bit?

If that is the case, then there would be no need for any timeout. Instead,
a qtest test can declare fail when the expected bit update is not observed by the test.

I would like to know.

Thanks,
Tong Ho

________________________________
From: Peter Maydell <peter.maydell@linaro.org>
Sent: Friday, October 27, 2023 6:03 AM
To: Ho, Tong <tong.ho@amd.com>
Cc: qemu-arm@nongnu.org <qemu-arm@nongnu.org>; qemu-devel@nongnu.org <qemu-devel@nongnu.org>; alistair@alistair23.me <alistair@alistair23.me>; edgar.iglesias@gmail.com <edgar.iglesias@gmail.com>; richard.henderson@linaro.org <richard.henderson@linaro.org>; frasse.iglesias@gmail.com <frasse.iglesias@gmail.com>
Subject: Re: [PATCH v4 3/3] tests/qtest: Introduce tests for AMD/Xilinx Versal TRNG device

On Tue, 17 Oct 2023 at 20:32, Tong Ho <tong.ho@amd.com> wrote:
>
> Signed-off-by: Tong Ho <tong.ho@amd.com>
> ---
>  tests/qtest/meson.build             |   2 +-
>  tests/qtest/xlnx-versal-trng-test.c | 486 ++++++++++++++++++++++++++++
>  2 files changed, 487 insertions(+), 1 deletion(-)
>  create mode 100644 tests/qtest/xlnx-versal-trng-test.c


> +static void trng_wait(uint32_t wait_mask, bool on, const char *act)
> +{
> +    time_t tmo = time(NULL) + 2; /* at most 2 seconds */

Don't put timeouts this short into tests, please; ideally,
avoid them entirely. Sometimes the CI machines are heavily
loaded and the test might not be able to complete in a
short time like this. If you must have a timeout, it should
be at least a minute. (And see below on whether we need one.)

> +    uint32_t event_mask = 0;
> +    uint32_t clear_mask = 0;
> +
> +    /*
> +     * Only selected bits are events in R_TRNG_STATUS, and
> +     * clear them needs to go through R_INT_CTRL.
> +     */
> +    if (wait_mask & TRNG_STATUS_CERTF_MASK) {
> +        event_mask |= TRNG_STATUS_CERTF_MASK;
> +        clear_mask |= TRNG_INT_CTRL_CERTF_RST_MASK;
> +    }
> +    if (wait_mask & TRNG_STATUS_DTF_MASK) {
> +        event_mask |= TRNG_STATUS_DTF_MASK;
> +        clear_mask |= TRNG_INT_CTRL_DTF_RST_MASK;
> +    }
> +    if (wait_mask & TRNG_STATUS_DONE_MASK) {
> +        event_mask |= TRNG_STATUS_DONE_MASK;
> +        clear_mask |= TRNG_INT_CTRL_DONE_RST_MASK;
> +    }
> +
> +    for (;;) {
> +        bool sta = !!(trng_status() & event_mask);
> +
> +        if ((on ^ sta) == 0) {
> +            break;
> +        }
> +
> +        if (time(NULL) >= tmo) {
> +            FAILED("%s: Timed out waiting for event 0x%x to be %d%s",
> +                   act, event_mask, (int)on, trng_info());
> +        }
> +
> +        g_usleep(10000);

Why does this test need to use sleeps and timeouts?
A qtest test controls the guest 'clock' directly, so
usually they're completely deterministic. Is there some
behaviour in the TRNG device which is asynchronous (in
a way not driven from the QEMU guest clock) that I've missed ?

> +    }
> +
> +    /* Remove event */
> +    trng_bit_set(R_TRNG_INT_CTRL, clear_mask);
> +
> +    if (!!(trng_read(R_TRNG_STATUS) & event_mask)) {
> +        FAILED("%s: Event 0x%0x stuck at 1 after clear: %s",
> +               act, event_mask, trng_info());
> +    }
> +}

thanks
-- PMM

[-- Attachment #2: Type: text/html, Size: 10197 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 3/3] tests/qtest: Introduce tests for AMD/Xilinx Versal TRNG device
  2023-10-30  6:25     ` Ho, Tong
@ 2023-10-30  9:57       ` Peter Maydell
  0 siblings, 0 replies; 10+ messages in thread
From: Peter Maydell @ 2023-10-30  9:57 UTC (permalink / raw)
  To: Ho, Tong
  Cc: qemu-arm@nongnu.org, qemu-devel@nongnu.org,
	alistair@alistair23.me, edgar.iglesias@gmail.com,
	richard.henderson@linaro.org, frasse.iglesias@gmail.com

On Mon, 30 Oct 2023 at 06:25, Ho, Tong <tong.ho@amd.com> wrote:
>
> Hi Peter,
>
> This is in regard to your comment on the use of g_usleep() in waiting for
> an event-bit update from the device under test.
>
> The TRNG device model presented in patch #1 does not have any asynchronous behavior.
>
> So, do you mean that, although the qtest process and the qemu-system-* process
> are running as 2 separate processes, qtest infrastructure already has the necessary
> mechanism to ensure that:
>
> 1. After qtest test sets a register that has the correct deterministic behavior to update an event bit,
>
> 2. The same qtest test subsequently issuing a register read will be guaranteed to observe the updated bit?

Yes. With a qtest test, there is no guest CPU inside QEMU. Instead
when the test says "write a register" there's a protocol between
the QEMU-under-test and the test process that says "write this
value to this address". Time within the simulation only advances
if the test specifically asks for it.

So if your device under test has the usual property of "we just
immediately update the state on register write" you're guarenteed
to see things on a subsequent read. If you are attempting a more
exact simulation and do things like "when this register is written
we set up a QEMU timer so that we only report the device ready
after a certain time" then your test needs to manually advance
the simulation clock with clock_step() to the point when the
device would report ready. (Usually we don't care about that
level of simulation accuracy, so clock_step() is used largely
in tests of timer devices.) Either way, this is unrelated to
wall-clock time.

thanks
-- PMM


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2023-10-30  9:58 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-17 19:32 [PATCH v4 0/3] AMD/Xilinx Versal TRNG support Tong Ho
2023-10-17 19:32 ` [PATCH v4 1/3] hw/misc: Introduce AMD/Xilix Versal TRNG device Tong Ho
2023-10-27 12:54   ` Peter Maydell
2023-10-17 19:32 ` [PATCH v4 2/3] hw/arm: xlnx-versal-virt: Add AMD/Xilinx " Tong Ho
2023-10-27 13:03   ` Peter Maydell
2023-10-17 19:32 ` [PATCH v4 3/3] tests/qtest: Introduce tests for AMD/Xilinx Versal " Tong Ho
2023-10-27 13:03   ` Peter Maydell
2023-10-30  6:25     ` Ho, Tong
2023-10-30  9:57       ` Peter Maydell
2023-10-27 13:15   ` Peter Maydell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).