[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v4 1/3] hw/misc: Introduce AMD/Xilix Versal TRNG device
From: |
Peter Maydell |
Subject: |
Re: [PATCH v4 1/3] hw/misc: Introduce AMD/Xilix Versal TRNG device |
Date: |
Fri, 27 Oct 2023 13:54:20 +0100 |
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