Re: [Qemu-devel] [PATCH v4 23/24] target/ppc: Use qemu_guest_getrandom for DARN

[Laurent Vivier]

On 06/05/2019 19:33, Richard Henderson wrote:
> We now have an interface for guest visible random numbers.
>
> Acked-by: David Gibson <address@hidden>
> Signed-off-by: Richard Henderson <address@hidden>
> ---
>   target/ppc/int_helper.c | 42 +++++++++++++++++++++++++++++------------
>   1 file changed, 30 insertions(+), 12 deletions(-)
>
> diff --git a/target/ppc/int_helper.c b/target/ppc/int_helper.c
> index f6a088ac08..9059e70b9c 100644
> --- a/target/ppc/int_helper.c
> +++ b/target/ppc/int_helper.c
> @@ -23,6 +23,8 @@
>   #include "exec/helper-proto.h"
>   #include "crypto/aes.h"
>   #include "fpu/softfloat.h"
> +#include "qapi/error.h"
> +#include "qemu/guest-random.h"
>   
>   #include "helper_regs.h"
>   
> /*****************************************************************************/
> @@ -158,25 +160,41 @@ uint32_t helper_cmpeqb(target_ulong ra, target_ulong rb)
>   #undef hasvalue
>   
>   /*
> - * Return invalid random number.
> - *
> - * FIXME: Add rng backend or other mechanism to get cryptographically suitable
> - * random number
> + * Return a random number.
>    */
> -target_ulong helper_darn32(void)
> +uint64_t helper_darn32(void)
>   {
> -    return -1;
> +    Error *err = NULL;
> +    uint32_t ret;
> +
> +    if (qemu_guest_getrandom(&ret, 4, &err) < 0) {

sizeof(ret)?

> +        qemu_log_mask(LOG_UNIMP, "darn: Crypto failure: %s",
> +                      error_get_pretty(err));
> +        error_free(err);
> +        return -1;
> +    }
> +
> +    return ret;
>   }
>   
> -target_ulong helper_darn64(void)
> +uint64_t helper_darn64(void)
>   {
> -    return -1;
> +    Error *err = NULL;
> +    uint64_t ret;
> +
> +    do {
> +        if (qemu_guest_getrandom(&ret, 8, &err) < 0) {

sizeof(ret)?

> +            qemu_log_mask(LOG_UNIMP, "darn: Crypto failure: %s",
> +                          error_get_pretty(err));
> +            error_free(err);
> +            return -1;
> +        }
> +        /* Since -1 is the error condition, try again for that case.  */
> +    } while (unlikely(ret == -1));

I think you don't need to do that, according to documentation, this is 
done by the software:

"Programming Note: When the error value is obtained, software is
expected to repeat the operation. [...]" - PowerISA Version 3.0B

Thanks,
Laurent