From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60791) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZhDPg-0002en-NU for qemu-devel@nongnu.org; Wed, 30 Sep 2015 05:11:01 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZhDPc-0000FB-L5 for qemu-devel@nongnu.org; Wed, 30 Sep 2015 05:11:00 -0400 References: <20150928101346.23919.3988.stgit@bahia.huguette.org> <20150929050109.GL19428@voom.redhat.com> <20150930103349.3772ab13@bahia.local> From: Thomas Huth Message-ID: <560BA71C.50703@redhat.com> Date: Wed, 30 Sep 2015 11:10:52 +0200 MIME-Version: 1.0 In-Reply-To: <20150930103349.3772ab13@bahia.local> Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="EH9VV5aRWugraXU6R5VT5SmhQITbv5CMe" Subject: Re: [Qemu-devel] [PATCH] spapr: add a default rng device List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Greg Kurz , David Gibson Cc: qemu-ppc@nongnu.org, qemu-devel@nongnu.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --EH9VV5aRWugraXU6R5VT5SmhQITbv5CMe Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 30/09/15 10:33, Greg Kurz wrote: > On Tue, 29 Sep 2015 15:01:09 +1000 > David Gibson wrote: >=20 >> On Mon, Sep 28, 2015 at 12:13:47PM +0200, Greg Kurz wrote: >>> A recent patch by Thomas Huth brought a new spapr-rng pseudo-device t= o >>> provide high-quality random numbers to guests. The device may either = be >>> backed by a "RngBackend" or the in-kernel implementation of the H_RAN= DOM >>> hypercall. >>> >>> Since modern POWER8 based servers always provide a hardware rng, it m= akes >>> sense to create a spapr-rng device with use-kvm=3Dtrue by default whe= n it >>> is available. >>> >>> Of course we want the user to have full control on how the rng is han= dled. >>> The default device WILL NOT be created in the following cases: >>> - the -nodefaults option was passed >>> - a spapr-rng device was already passed on the command line >>> >>> The default device is created at reset time to ensure devices specifi= ed on >>> the command line have been created. >>> >>> Signed-off-by: Greg Kurz >> >> So, I think the concept is ok, but.. >> >=20 > Just to be sure about the concept. >=20 > The goal is to free users from having to explicitely pass >=20 > -device spapr-rng,use-kvm=3Dtrue >=20 > ... when ALL the following conditions are met: >=20 > 1) KVM is used and advertises KVM_CAP_PPC_HWRNG > 2) -nodefaults HAS NOT been passed on the cmdline > 3) -device spapr-rng HAS NOT been passed on the cmdline >=20 >>> --- >>> hw/ppc/spapr.c | 17 +++++++++++++++++ >>> hw/ppc/spapr_rng.c | 2 +- >>> target-ppc/kvm.c | 9 +++++---- >>> target-ppc/kvm_ppc.h | 6 ++++++ >>> 4 files changed, 29 insertions(+), 5 deletions(-) >>> >>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c >>> index 7f4f196e53e5..ee048ecffd0c 100644 >>> --- a/hw/ppc/spapr.c >>> +++ b/hw/ppc/spapr.c >>> @@ -1059,6 +1059,14 @@ static int spapr_check_htab_fd(sPAPRMachineSta= te *spapr) >>> return rc; >>> } >>> =20 >>> +static void spapr_rng_create(void) >>> +{ >>> + Object *rng =3D object_new(TYPE_SPAPR_RNG); >>> + >>> + object_property_set_bool(rng, true, "use-kvm", &error_abort); >>> + object_property_set_bool(rng, true, "realized", &error_abort); >>> +} >>> + >>> static void ppc_spapr_reset(void) >>> { >>> sPAPRMachineState *spapr =3D SPAPR_MACHINE(qdev_get_machine()); >>> @@ -1082,6 +1090,15 @@ static void ppc_spapr_reset(void) >>> spapr->rtas_addr =3D rtas_limit - RTAS_MAX_SIZE; >>> spapr->fdt_addr =3D spapr->rtas_addr - FDT_MAX_SIZE; >>> =20 >>> + /* Create a rng device if the user did not provide it already an= d >>> + * KVM has hwrng support. >>> + */ >>> + if (defaults_enabled() && >>> + kvmppc_hwrng_present() && >>> + !object_resolve_path_type("", TYPE_SPAPR_RNG, NULL)) { >>> + spapr_rng_create(); >>> + } >>> + >> >> Constructing the RNG at reset time is just wrong. Using >> defaults_enabled() is ugly at the best of times, using it at reset, >> after construction of the qom tree is generally complete, is just >> hideous. >> >=20 > Yeah I ended up with this hack because I could not figure out how > to give priority to a spapr-rng device specified on the cmdline > over the automatic one... poor QOM skills :\ >=20 > If you have a suggestion to handle this case in a more appropriate way,= > and it is worth the pain compared to the gain, please advice. Not sure whether this might be an acceptable solution, but maybe you could use qemu_opts_foreach(qemu_find_opts("device"), ...) to check whether a "spapr-rng" device has been specified at the command line? Thomas --EH9VV5aRWugraXU6R5VT5SmhQITbv5CMe 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) iQIcBAEBAgAGBQJWC6ccAAoJEC7Z13T+cC21EucP/25WEFy9U7FQlmpXeclMOgvn L299MDDXrsgf40RQtkowGJLL6Aa+F7222jZJ2C0iAgp6OxGZau5FEYbDd1bLQIey 0fhJUTO5FLBrTPd3Z5YFlsZr0gumJ6uE8uaV/dgPTJ0iUjRlkOIFsC2dGz+rzQWF +cqGgrznwZrW/qSACRDlwwv8eafEbJN2cx+6f7lPZsArTyNvEJ2Sj3wwIg9kVFhI L7JOTUDWz8IUHdYpDOO3uTScq/DWCVQPb/vMK9EJtDp3/Bagv2nfdymvxBIPNgai cggerouq6X0mjf4wcT7uwqWBI0aRV2VVw/0RCP8tp/uv0KdZqknASLGpb1Srk0c/ GHA9SUbhCl0hGv47EWEP8tjx/NnKNxrKq3NJkpUhvMaygqMTrb38yZNcglV8tY3s nRzOBlhyhsWEPn/ETBfsBNyiOl2Dh0Q6VQLbGXj+FYUK6jrBsxh+jzz6umsEAVOP QLJMQYqXpDs/j7r/Nhla8VjHT35/TAQz5inmfiTQtAndlxeldmGQ/LBiRlkVoTsO yJFxYkbLL8Lbyg0c+Foo6TCLmxUILgHpiwGIAHi2o2s4CkMGSD3Zk8Xz29PFLXfu xdmY8YLV/StZfuu9INmWH5lv8pjbtRlwA+vjLK+6Zw49aojxcl2NxITEyHOiHT1v +Hb+vWgL4jWwDiTs9Z0w =8V2d -----END PGP SIGNATURE----- --EH9VV5aRWugraXU6R5VT5SmhQITbv5CMe--