From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56607) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZYxzh-0003o7-LR for qemu-devel@nongnu.org; Mon, 07 Sep 2015 11:06:06 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZYxzc-0002Z7-II for qemu-devel@nongnu.org; Mon, 07 Sep 2015 11:06:05 -0400 References: <1441046762-5788-1-git-send-email-thuth@redhat.com> <1441046762-5788-3-git-send-email-thuth@redhat.com> <20150901004753.GJ11475@voom.redhat.com> From: Thomas Huth Message-ID: <55EDA7CC.4000905@redhat.com> Date: Mon, 7 Sep 2015 17:05:48 +0200 MIME-Version: 1.0 In-Reply-To: <20150901004753.GJ11475@voom.redhat.com> Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="PmvvCWaWK9qTPN5xxs17mPA8h2nXM3I6L" Subject: Re: [Qemu-devel] [PATCH v2 2/2] ppc/spapr_hcall: Implement H_RANDOM hypercall in QEMU List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Gibson Cc: qemu-devel@nongnu.org, agraf@suse.de, armbru@redhat.com, michael@ellerman.id.au, qemu-ppc@nongnu.org, amit.shah@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --PmvvCWaWK9qTPN5xxs17mPA8h2nXM3I6L Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable 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=3Drng-random ... =2E.. >> 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; >> } >> =20 >> +typedef struct HRandomData { >> + QemuSemaphore sem; >> + union { >> + uint64_t v64; >> + uint8_t v8[8]; >> + } val; >> + int received; >> +} HRandomData; >> + >> +static RndRandom *hrandom_rng; >=20 > Couldn't you avoid the new global by looking this up through the > sPAPRMachineState? >=20 >> + >> +static void random_recv(void *dest, const void *src, size_t size) >> +{ >> + HRandomData *hrcrdp =3D dest; >> + >> + if (src && size > 0) { >> + memcpy(&hrcrdp->val.v8[hrcrdp->received], src, size); >=20 > I'd be happier with an assert() ensuring that size doesn't exceed the > buffer space we have left. >=20 >> + hrcrdp->received +=3D size; >> + } >> + qemu_sem_post(&hrcrdp->sem); >=20 > 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). 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. >> +} >> + >> +static target_ulong h_random(PowerPCCPU *cpu, sPAPRMachineState *spap= r, >> + target_ulong opcode, target_ulong *args)= >> +{ >> + HRandomData hrcrd; >> + >> + if (!hrandom_rng) { >> + return H_HARDWARE; >> + } >> + >> + qemu_sem_init(&hrcrd.sem, 0); >> + hrcrd.val.v64 =3D 0; >> + hrcrd.received =3D 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] =3D hrcrd.val.v64; >> + >> + return H_SUCCESS; >> +} I'll post a new version with the other changes soon. Thomas --PmvvCWaWK9qTPN5xxs17mPA8h2nXM3I6L Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) iQIcBAEBAgAGBQJV7afSAAoJEC7Z13T+cC21RcMP/178FfsBPpVd+guWZUlkwzRA c7AUSO9k5k3n5FSza7BID8vh36HR9MGR4hQDgSsIlkrsv2LSmPPQRAx+6EVj4Z4g 9LWnsCD300GLO75I7FdE587RZ8qhJMCaXAXub4ym9c25JwJrPtKGzYfCWxk8EE3I Agw2VpNy5EhzHv1q97/R7JUt5ki2ywkj4hcvjmsQtBUcHQs6ker6b6yCu195kCj+ 6ZNrojqets6H6YXWMrN31A+pmSMLZE277TszvZe+EElISniGN0Z0tLUfCH+7kAIm EB0fwC9kHwxAbW6hh8UAM5of2XPq/T0OOxRMyphwswNEDciZDnBrTu5OALRAMXBK ihFRT4ArjURm9OKgc9qnWjI1TorVJ+kwO8Q0FFK4kbx6uKP22sZGTwNa6mwyxWHi QZFeCvsmUINSOi/CrSu1pm2MngDhgWjfeZQJC+UK8US3hNdN0hUAuNcRb2bRHAe5 ke0YDqKt1aX8upwz4s+R0W+jDh0aGiFU8IpmUf1mHnJFzesJR1iFBLTfrSIfDgkE s9QGBd1vN4BVY+Z+A4tEk7y2cfTOdZrzZPctRk7BoJaes3ntrKv8qCaaOcs4BO2Z R8K28V7+++jfTuJUbvJaDtJGU8Et/Yl1FNsz8rH1DQyBuj73L0tsbDrXnxmUscXz iv44NaDeb9Bajgl7lxA9 =TmZo -----END PGP SIGNATURE----- --PmvvCWaWK9qTPN5xxs17mPA8h2nXM3I6L--