On Mon, Sep 07, 2015 at 05:05:48PM +0200, Thomas Huth wrote:
> On 01/09/15 02:47, David Gibson wrote:
> > On Mon, Aug 31, 2015 at 08:46:02PM +0200, Thomas Huth wrote:
> >> The PAPR interface provides a hypercall to pass high-quality
> >> hardware generated random numbers to guests. So let's provide
> >> this call in QEMU, too, so that guests that do not support
> >> virtio-rnd yet can get good random numbers, too.
> >> Please note that this hypercall should provide "good" random data
> >> instead of pseudo-random, so the function uses the RngBackend to
> >> retrieve the values instead of using a "simple" library function
> >> like rand() or g_random_int(). Since there are multiple RngBackends
> >> available, the user must select an appropriate backend via the
> >> "h-random" property of the the machine state to enable it, e.g.
> >>
> >> qemu-system-ppc64 -M pseries,h-random=rng-random ...
> ...
> >> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> >> index 652ddf6..ff9d4fd 100644
> >> --- a/hw/ppc/spapr_hcall.c
> >> +++ b/hw/ppc/spapr_hcall.c
> >> @@ -1,4 +1,8 @@
> >> #include "sysemu/sysemu.h"
> >> +#include "sysemu/rng.h"
> >> +#include "sysemu/rng-random.h"
> >> +#include "qom/object_interfaces.h"
> >> +#include "qemu/error-report.h"
> >> #include "cpu.h"
> >> #include "helper_regs.h"
> >> #include "hw/ppc/spapr.h"
> >> @@ -929,6 +933,77 @@ static target_ulong
> >> h_client_architecture_support(PowerPCCPU *cpu_,
> >> return H_SUCCESS;
> >> }
> >>
> >> +typedef struct HRandomData {
> >> + QemuSemaphore sem;
> >> + union {
> >> + uint64_t v64;
> >> + uint8_t v8[8];
> >> + } val;
> >> + int received;
> >> +} HRandomData;
> >> +
> >> +static RndRandom *hrandom_rng;
> >
> > Couldn't you avoid the new global by looking this up through the
> > sPAPRMachineState?
> >
> >> +
> >> +static void random_recv(void *dest, const void *src, size_t size)
> >> +{
> >> + HRandomData *hrcrdp = dest;
> >> +
> >> + if (src && size > 0) {
> >> + memcpy(&hrcrdp->val.v8[hrcrdp->received], src, size);
> >
> > I'd be happier with an assert() ensuring that size doesn't exceed the
> > buffer space we have left.
> >
> >> + hrcrdp->received += size;
> >> + }
> >> + qemu_sem_post(&hrcrdp->sem);
> >
> > Could you avoid a few wakeups by only posting the semaphore once the
> > buffer is filled?
>
> I tried that now, but calling rng_backend_request_entropy() from within
> the callback function does not work (since entropy_available() in
> rng-random.c clears the callback function variable after having called
> the callback).
Ah, ok. I'd missed the fact that rng_backend_request_entropy() was
being called again - I'd assumed that after the first call the backend
would just keep notifying until you had as much data as requested.
> And since you normally seem get 8 bytes in the first shot already anyway
> when using a good random number generator source, I think it's best to
> simply keep the logic as I've currently got it - at least that's easiest
> to understand when reading the source code.
Yes, I concur.
>
> >> +}
> >> +
> >> +static target_ulong h_random(PowerPCCPU *cpu, sPAPRMachineState *spapr,
> >> + target_ulong opcode, target_ulong *args)
> >> +{
> >> + HRandomData hrcrd;
> >> +
> >> + if (!hrandom_rng) {
> >> + return H_HARDWARE;
> >> + }
> >> +
> >> + qemu_sem_init(&hrcrd.sem, 0);
> >> + hrcrd.val.v64 = 0;
> >> + hrcrd.received = 0;
> >> +
> >> + qemu_mutex_unlock_iothread();
> >> + while (hrcrd.received < 8) {
> >> + rng_backend_request_entropy((RngBackend *)hrandom_rng,
> >> + 8 - hrcrd.received, random_recv,
> >> &hrcrd);
> >> + qemu_sem_wait(&hrcrd.sem);
> >> + }
> >> + qemu_mutex_lock_iothread();
> >> +
> >> + qemu_sem_destroy(&hrcrd.sem);
> >> + args[0] = hrcrd.val.v64;
> >> +
> >> + return H_SUCCESS;
> >> +}
>
> I'll post a new version with the other changes soon.
>
> Thomas
>
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
pgpPWyU2IIGLD.pgp
Description: PGP signature
