From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39398) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZaIn9-00069W-1u for qemu-devel@nongnu.org; Fri, 11 Sep 2015 03:30:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZaIn4-0004hF-Nt for qemu-devel@nongnu.org; Fri, 11 Sep 2015 03:30:38 -0400 References: <1441046762-5788-1-git-send-email-thuth@redhat.com> <1441046762-5788-2-git-send-email-thuth@redhat.com> <20150901003808.GI11475@voom.redhat.com> <55E583A6.4000600@redhat.com> <20150908050316.GA372@tungsten.ozlabs.ibm.com> <20150908051528.GD24774@voom.redhat.com> <55F0A036.2090508@redhat.com> <55F13241.8040004@redhat.com> <20150910104051.GC11781@voom> <55F1719B.5070208@redhat.com> <20150911004527.GF11781@voom> From: Thomas Huth Message-ID: <55F28314.7070608@redhat.com> Date: Fri, 11 Sep 2015 09:30:28 +0200 MIME-Version: 1.0 In-Reply-To: <20150911004527.GF11781@voom> Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="UXV4iA8Xqgam8GOOeD20ehiIwwwCS3WoG" Subject: Re: [Qemu-devel] [Qemu-ppc] [PATCH v2 1/2] spapr: Add support for hwrng when available List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Gibson Cc: Alexander Graf , armbru@redhat.com, qemu-devel@nongnu.org, michael@ellerman.id.au, qemu-ppc@nongnu.org, amit.shah@redhat.com, Sam Bobroff This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --UXV4iA8Xqgam8GOOeD20ehiIwwwCS3WoG Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 11/09/15 02:45, David Gibson wrote: > On Thu, Sep 10, 2015 at 02:03:39PM +0200, Thomas Huth wrote: >> On 10/09/15 12:40, David Gibson wrote: >>> On Thu, Sep 10, 2015 at 09:33:21AM +0200, Thomas Huth wrote: >>>> On 09/09/15 23:10, Thomas Huth wrote: >>>>> On 08/09/15 07:15, David Gibson wrote: >>>> ... >>>>>> At this point rather than just implementing them as discrete machi= ne >>>>>> options, I suspect it will be more maintainable to split out the >>>>>> h-random implementation as a pseudo-device with its own qdev and s= o >>>>>> forth. We already do similarly for the RTAS time of day functions= >>>>>> (spapr-rtc). >>>>> >>>>> I gave that I try, but it does not work as expected. To be able to >>>>> specify the options, I'd need to instantiate this device with the >>>>> "-device" option, right? Something like: >>>>> >>>>> -device spapr-rng,backend=3Drng0,usekvm=3D0 >>>>> >>>>> Now this does not work when I use TYPE_SYS_BUS_DEVICE as parent cla= ss >>>>> like it is done for spapr-rtc, since the user apparently can not pl= ug >>>>> device to this bus on machine spapr (you can also not plug an spapr= -rtc >>>>> device this way!). >>>>> >>>>> The spapr-vlan, spapr-vty, etc. devices are TYPE_VIO_SPAPR_DEVICE, = so I >>>>> also tried that instead, but then the rng device suddenly shows up = under >>>>> /vdevice in the device tree - that's also not what we want, I guess= =2E >>>> >>>> I did some more tests, and I think I can get this working with one s= mall >>>> modification to spapr_vio.c >> ... >>>> i.e. when the dt_name has not been set, the device won't be added to= the >>>> /vdevice device tree node. If that's acceptable, I'll continue with = this >>>> approach. >>> >>> A bit hacky. >>> >>> I think it would be preferable to build it under SysBus by default, >>> like spapr-rtc. Properties can be set on the device using -global (o= r >>> -set, but -global is easier). >> >> If anyhow possible, I'd prefere to use "-device" for this instead, bec= ause >> >> a) it's easier to use for the user, for example you can simply use >> "-device spapr-rng,?" to get the list of properties - this >> does not seem to work with spapr-rtc (it has a "date" property >> which does not show up in the help text?) >=20 > Actually, I don't think that's got anything to do with -device versus > otherwise. "date" doesn't appear because it's an "object" property > rather than a "qdev" property - that distinction is subtle and > confusing, yes. At least it is not very friendly for the user ... if a configuration property does not show up in the help text, you've got to document it somewhere else or nobody will be aware of it. >> b) unlike the rtc device which is always instantiated, the rng >> device is rather optional, so it is IMHO more intuitive if >> created via the -device option. >=20 > Hrm, that's true though. And.. we're back at the perrenial question > of what "standard" devices should be constructed by default. And what > "default" means. >=20 > It seems to me that while the random device is optional, it should be > created by default. But with -device there's not really a way to do > that. But then again if it's constructed internally there's not > really a way to turn it off short of hacky machine options. Ugh. >=20 >> So I'd like to give it a try with the TYPE_VIO_SPAPR_DEVICE first ... = if >> you then still don't like the patches at all, I can still rework them = to >> use TYPE_SYS_BUS_DEVICE instead. >=20 > I still dislike putting it on the VIO "bus", since PAPR doesn't > consider it a VIO device. Hmm, that's also a valid point. After doing some more research, I think I've found yet another possibility (why isn't there a proper documentation/howto for all this QOM stuff ... or did I just miss it?) : Instead of using a bus, simply set parent =3D TYPE_DEVICE, so that it is = a "bus-less" device. Seems to work fine at a first glance, so unless somebody tells me that this is a very bad idea, I'll try to rework my patches accordingly... Thomas --UXV4iA8Xqgam8GOOeD20ehiIwwwCS3WoG 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) iQIcBAEBAgAGBQJV8oMUAAoJEC7Z13T+cC21MLYP/262pBSTAiiamyON4WJuo59S Uc1/2+/iS5WE7BpA+ia0T3NZxDDRSO5oIdkVSpMzq6PjzUF4ofzmNlh+90/cA8ic ylHEvhzgkjt8hRGuOVx2fksPg0L7xOZb9kMwamdPzOrVpVjRTd7rUcfWehFec6PG sgkhnkLazt2ArabG6Dy+7peGddye08k53Qtz/brbt492AkyIYTbvrT8J5mp3JOAh JWt0x/jKoEzMpnFxSQXhMzUk4UwIfQpNtVQT/qDbgfXhfL2J/7ZvWiLwDeXvkZQG 4L3v48booQXB4EPkf2qTDEXSDd2KN6QntWFnqjGTx464/pWr9OWWDXaS9c+6Zf2n rGAWjpnTmExu1yAp3rvSuOiL8IlYAAOhprp8DTtLpTWjbOX9r9hQZblfykZQZXY4 eoPEluaFtkXZrncMwpR4GnptbzJHX/M1oWWiDWpMnVsfUJuh+xKCGesNHGBJF39Z eCaNHn/dTKFiYMYMPHocbWZBHTn4OJR0Q3XXVYRw2OH4Jtt2rapFmNIQE6gLOx43 wSqHLlT/qreebxkM38wdROUn3VEBSin2wu92A/UguNKFZDxb+hVD5oDtynBk8W/4 jMTIA9ZrQkvfLxX2oYrX8/hn/tx5TRHRoCF2ZHJbMmT46NsLze/48+STWauuftyH 4UZrW4wpjaQrRCrb0T4r =kSnx -----END PGP SIGNATURE----- --UXV4iA8Xqgam8GOOeD20ehiIwwwCS3WoG--