Re: [Qemu-arm] [PATCH v3] aspeed_scu: Implement RNG register

[Peter Maydell]

On 29 May 2018 at 05:19, Joel Stanley <address@hidden> wrote:
> The ASPEED SoCs contain a single register that returns random data when
> read. This models that register so that guests can use it.
>
> The random number data register has a corresponding control register,
> however it returns data regardless of the state of the enabled bit, so
> the model follows this behaviour.
>
> Reviewed-by: Cédric Le Goater <address@hidden>
> Signed-off-by: Joel Stanley <address@hidden>
> ---
> v2:
>  - Remove call to qcrypto_random_init as this is done in main()
> v3:
>  - Add Cédric's reviewed-by
>  - Add a comment about why we don't check for the rng enable bit
> ---
>  hw/misc/aspeed_scu.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
>
> diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c
> index 5e6d5744eeca..96db052389cc 100644
> --- a/hw/misc/aspeed_scu.c
> +++ b/hw/misc/aspeed_scu.c
> @@ -16,6 +16,7 @@
>  #include "qapi/visitor.h"
>  #include "qemu/bitops.h"
>  #include "qemu/log.h"
> +#include "crypto/random.h"
>  #include "trace.h"
>
>  #define TO_REG(offset) ((offset) >> 2)
> @@ -154,6 +155,18 @@ static const uint32_t 
> ast2500_a1_resets[ASPEED_SCU_NR_REGS] = {
>       [BMC_DEV_ID]      = 0x00002402U
>  };
>
> +static uint32_t aspeed_scu_get_random(void)
> +{
> +    Error *err = NULL;
> +    uint32_t num;
> +
> +    if (qcrypto_random_bytes((uint8_t *)&num, sizeof(num), &err)) {
> +        error_report_err(err);
> +    }
> +
> +    return num;
> +}

This will return an uninitialized value if qcrypto_random_bytes()
fails.

Does the guest use this value for cryptographic purposes?
In hw/misc/bcm2835_rng.c we opted to make qcrypto_random_bytes()
failing be fatal (for reasons noted in the comment there),
though that is a bit rough. exynos4210_rng() is more conveniently
able to just never report to the guest that the PRNG is ready.

thanks
-- PMM