From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51117) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZWZjl-000128-Tg for qemu-devel@nongnu.org; Mon, 31 Aug 2015 20:47:47 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZWZjk-00059m-2S for qemu-devel@nongnu.org; Mon, 31 Aug 2015 20:47:45 -0400 Date: Tue, 1 Sep 2015 10:47:53 +1000 From: David Gibson Message-ID: <20150901004753.GJ11475@voom.redhat.com> References: <1441046762-5788-1-git-send-email-thuth@redhat.com> <1441046762-5788-3-git-send-email-thuth@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="liqSWPDvh3eyfZ9k" Content-Disposition: inline In-Reply-To: <1441046762-5788-3-git-send-email-thuth@redhat.com> 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: Thomas Huth Cc: qemu-devel@nongnu.org, agraf@suse.de, armbru@redhat.com, michael@ellerman.id.au, qemu-ppc@nongnu.org, amit.shah@redhat.com --liqSWPDvh3eyfZ9k Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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. >=20 > qemu-system-ppc64 -M pseries,h-random=3Drng-random ... I think it would be a better idea to require that the backend RNG be constructed with -object, then give its id to the pseries code. That matches what's needed for virtio-rng more closely, and also makes it simpler to supply parameters to the RNG backend, if necessary. There's also a wart in that whatever the user specifies by way of backend, it will be silently overridden if KVM does implement the H_RANDOM call. I'm not sure how best to handle that. > to use the /dev/random backend, or "h-random=3Drng-egd" to use the > Entropy Gathering Daemon instead. >=20 > Signed-off-by: Thomas Huth > --- > hw/ppc/spapr.c | 36 +++++++++++++++++++++--- > hw/ppc/spapr_hcall.c | 75 ++++++++++++++++++++++++++++++++++++++++++++= ++++++ > include/hw/ppc/spapr.h | 2 ++ > 3 files changed, 109 insertions(+), 4 deletions(-) >=20 > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index bc3a112..3db87b5 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -312,7 +312,8 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base, > hwaddr kernel_size, > bool little_endian, > const char *kernel_cmdline, > - uint32_t epow_irq) > + uint32_t epow_irq, > + bool enable_h_random) > { > void *fdt; > uint32_t start_prop =3D cpu_to_be32(initrd_base); > @@ -466,7 +467,7 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base, > =20 > _FDT((fdt_end_node(fdt))); > =20 > - if (kvmppc_hwrng_present()) { > + if (enable_h_random) { > _FDT(fdt_begin_node(fdt, "ibm,platform-facilities")); > =20 > _FDT(fdt_property_string(fdt, "name", "ibm,platform-facilities")= ); > @@ -1472,7 +1473,7 @@ static void ppc_spapr_init(MachineState *machine) > uint32_t initrd_base =3D 0; > long kernel_size =3D 0, initrd_size =3D 0; > long load_limit, fw_size; > - bool kernel_le =3D false; > + bool kernel_le =3D false, enable_h_random; I'd prefer to have the new variable on a new line - this way makes it very easy to miss the initializer on the old one. > char *filename; > =20 > msi_supported =3D true; > @@ -1699,6 +1700,10 @@ static void ppc_spapr_init(MachineState *machine) > } > g_free(filename); > =20 > + /* Enable H_RANDOM hypercall if requested by user or supported by ke= rnel */ > + enable_h_random =3D kvmppc_hwrng_present() || > + !spapr_h_random_init(spapr->h_random_type); > + > /* FIXME: Should register things through the MachineState's qdev > * interface, this is a legacy from the sPAPREnvironment structure > * which predated MachineState but had a similar function */ > @@ -1710,7 +1715,8 @@ static void ppc_spapr_init(MachineState *machine) > spapr->fdt_skel =3D spapr_create_fdt_skel(initrd_base, initrd_size, > kernel_size, kernel_le, > kernel_cmdline, > - spapr->check_exception_irq); > + spapr->check_exception_irq, > + enable_h_random); > assert(spapr->fdt_skel !=3D NULL); > =20 > /* used by RTAS */ > @@ -1810,6 +1816,21 @@ static void spapr_set_kvm_type(Object *obj, const = char *value, Error **errp) > spapr->kvm_type =3D g_strdup(value); > } > =20 > +static char *spapr_get_h_random_type(Object *obj, Error **errp) > +{ > + sPAPRMachineState *spapr =3D SPAPR_MACHINE(obj); > + > + return g_strdup(spapr->h_random_type); > +} > + > +static void spapr_set_h_random_type(Object *obj, const char *val, Error = **errp) > +{ > + sPAPRMachineState *spapr =3D SPAPR_MACHINE(obj); > + > + g_free(spapr->h_random_type); > + spapr->h_random_type =3D g_strdup(val); > +} > + > static void spapr_machine_initfn(Object *obj) > { > object_property_add_str(obj, "kvm-type", > @@ -1817,6 +1838,13 @@ static void spapr_machine_initfn(Object *obj) > object_property_set_description(obj, "kvm-type", > "Specifies the KVM virtualization mo= de (HV, PR)", > NULL); > + > + object_property_add_str(obj, "h-random", spapr_get_h_random_type, > + spapr_set_h_random_type, NULL); > + object_property_set_description(obj, "h-random", > + "Select RNG backend for H_RANDOM hyp= ercall " > + "(rng-random, rng-egd)", > + NULL); > } > =20 > static void ppc_cpu_do_nmi_on_cpu(void *arg) > 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(Po= werPCCPU *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; 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 =3D 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 +=3D size; > + } > + qemu_sem_post(&hrcrdp->sem); Could you avoid a few wakeups by only posting the semaphore once the buffer is filled? > +} > + > +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 =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, &hr= crd); > + qemu_sem_wait(&hrcrd.sem); > + } > + qemu_mutex_lock_iothread(); > + > + qemu_sem_destroy(&hrcrd.sem); > + args[0] =3D hrcrd.val.v64; > + > + return H_SUCCESS; > +} > + > +int spapr_h_random_init(const char *backend_type) > +{ > + Error *local_err =3D NULL; > + > + if (!backend_type) { > + return -1; > + } > + > + hrandom_rng =3D RNG_RANDOM(object_new(backend_type)); > + user_creatable_complete(OBJECT(hrandom_rng), &local_err); > + if (local_err) { > + error_printf("Failed to initialize backend for H_RANDOM:\n %s\n= ", > + error_get_pretty(local_err)); > + object_unref(OBJECT(hrandom_rng)); > + return -1; > + } > + > + spapr_register_hypercall(H_RANDOM, h_random); > + > + return 0; > +} > + > static spapr_hcall_fn papr_hypercall_table[(MAX_HCALL_OPCODE / 4) + 1]; > static spapr_hcall_fn kvmppc_hypercall_table[KVMPPC_HCALL_MAX - KVMPPC_H= CALL_BASE + 1]; > =20 > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h > index ab8906f..de17624 100644 > --- a/include/hw/ppc/spapr.h > +++ b/include/hw/ppc/spapr.h > @@ -76,6 +76,7 @@ struct sPAPRMachineState { > =20 > /*< public >*/ > char *kvm_type; > + char *h_random_type; > }; > =20 > #define H_SUCCESS 0 > @@ -592,6 +593,7 @@ int spapr_tcet_dma_dt(void *fdt, int node_off, const = char *propname, > void spapr_pci_switch_vga(bool big_endian); > void spapr_hotplug_req_add_event(sPAPRDRConnector *drc); > void spapr_hotplug_req_remove_event(sPAPRDRConnector *drc); > +int spapr_h_random_init(const char *backend_type); > =20 > /* rtas-configure-connector state */ > struct sPAPRConfigureConnectorState { --=20 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 --liqSWPDvh3eyfZ9k Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJV5PW5AAoJEGw4ysog2bOSZGEP/iHMJSfGS6FxdMeWGYeH64ar PkwNvXS2VMkn3sJJT2wOHgdJkt9z7HEKD2UqjOImnKSCW1oFt5in5W7t1vtOU46a h9aS/1L3dCXchRlO4lQX5uq68YvQNF2MoqgT61IwANOujuumjDduZPzFclH+W+Eu 3odRmMllGBs3/YqRw8m2E2R4AaYZbsrFlT/ktDJPQO2Pducalr20q/mmWiQMP3ad NL+Log8ABxt4ZycO8RNy5F479crlHa3Uk9ArR+Mg5pJPBPeFsXOcLb7POVrlh4xF iVcaIz83va6IxenHiacgn96ViWxrvFF0HXIGXpO4ltN7jjV/ZHFP2w3M1CIqeIC2 mIQ57c5gdWN4y+2b98mVHLOVPC6WJ+lGfPfqkZ6L4WsOnE3tTqDexMAdmnYPbNSW KOBhaMYExr9+F4rlXVyStYymwt7sRJuXkMvbw2hiywcrB2dqCMHZGxylaltZTu9D hBjicsd8sL7meqi/9Fevg+dggS+YXTy/tOZY/j5OWdh0ZtylQsJ1UF5+C1GWnc0T VyiAxDQV5VyGYn0kzcYZXfaykT44WHgHEZ5F2FtcE/I2FbAOK2eNN71apVcpNzl9 3OCJRRKWoG1XIX7v9IKqbQuQBfgwjB088zTRrZXaHQ3V5mNZJo804xiVSlRNKouw kBrzA9WOds4MLIl+koF9 =mlZr -----END PGP SIGNATURE----- --liqSWPDvh3eyfZ9k--