From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55747) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZbJTv-0003Nw-7R for qemu-devel@nongnu.org; Sun, 13 Sep 2015 22:27:00 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZbJTs-0002CV-04 for qemu-devel@nongnu.org; Sun, 13 Sep 2015 22:26:59 -0400 Date: Mon, 14 Sep 2015 12:27:05 +1000 From: David Gibson Message-ID: <20150914022705.GF2547@voom.fritz.box> References: <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> <20150911004657.GG11781@voom> <55F2A226.5040407@suse.de> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="Ls2Gy6y7jbHLe9Od" Content-Disposition: inline In-Reply-To: <55F2A226.5040407@suse.de> 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: Alexander Graf Cc: Thomas Huth , "armbru@redhat.com" , "qemu-devel@nongnu.org" , "michael@ellerman.id.au" , "qemu-ppc@nongnu.org" , "amit.shah@redhat.com" , Sam Bobroff --Ls2Gy6y7jbHLe9Od Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Sep 11, 2015 at 11:43:02AM +0200, Alexander Graf wrote: >=20 >=20 > On 11.09.15 02:46, David Gibson wrote: > > On Thu, Sep 10, 2015 at 02:13:26PM +0200, Alexander Graf wrote: > >> > >> > >>> Am 10.09.2015 um 14:03 schrieb Thomas Huth : > >>> > >>>> 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 mach= ine > >>>>>>> options, I suspect it will be more maintainable to split out the > >>>>>>> h-random implementation as a pseudo-device with its own qdev and = so > >>>>>>> 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 cl= ass > >>>>>> like it is done for spapr-rtc, since the user apparently can not p= lug > >>>>>> device to this bus on machine spapr (you can also not plug an spap= r-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 gues= s. > >>>>> > >>>>> I did some more tests, and I think I can get this working with one = small > >>>>> modification to spapr_vio.c > >>> ... > >>>>> i.e. when the dt_name has not been set, the device won't be added t= o 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 (= or > >>>> -set, but -global is easier). > >>> > >>> If anyhow possible, I'd prefere to use "-device" for this instead, be= cause > >>> > >>> 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?) > >>> > >>> 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. > >>> > >>> 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. > >> > >> Please don't use sysbus. If the vio device approach turns ugly, > >> create a new spapr hcall bus instead. We should have had that from > >> the beginning really. > >=20 > > Ok.. why? > >=20 > > It's a system (pseudo-)device that doesn't have any common bus > > infrastructure with anything else. Isn't that what SysBus is for? >=20 > No, sysbus means "A device that has MMIO and/or PIO connected via a bus > I'm too lazy to model" really. These devices have neither. Oh. So.. where is one supposed to find that out? > Back in the days before QOM, sysbus was our lowest common denominator, > but now that we have TYPE_DEVICE and can branch off of that, we really > shouldn't abuse sysbus devices for things they aren't. So what actually is the root of the qdev tree then? --=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 --Ls2Gy6y7jbHLe9Od Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJV9jB5AAoJEGw4ysog2bOSXnsQAJnpW82xGo4r9fquBHUk/8Ji 28/6Qn3NcVdr7UJm9zV4338qP03VUG9zJgy3IKTXgojipawa5wa2YsHDJGfdTod4 i4/E9akOMnIEaUJTHmu1mrIcxhNk5MdOghs4nJ3340Z9fvMluUpvKi2/GH6ltjF8 XNRFPU4JAA1MhaidoX+Ar62/igd6Zc7jN0AA77kqdgGKavAD5KRIfDLQOLZM4jR+ 8TFmGdAzXa7foCSpqVeEBqa5+GcH+kHcZZrISv/aSRAyCTappFjFV+MxHpfeawdq F/ljyxR8UtpTIIC5GCMRQJ0pRYLbTicXkabQJ5VynXjBmJv2DQSJwYJw0rhjSxee Vi/6LLH0/Ac6oJDoBrs1V+P4AJ9B0WCJs1ofgFihLARAcq7/wmK/vrpWpeSvO1Tz qBis7V9I7/07Vnu/MCIuMbA4QVPsFsm+ZmC2NpD6W98LOOQQ1He45RBaNFHFwbLZ CTsvfVcILaGTiBXbg0f5wVbBBfzKrPz6SkHdEoD3vdVEUHuv0xOx9cbT09X1rLV0 AG+bIoWwoNKqeg8HqeUOqKSjkpo3fCU98NV3hbV4I+w9Y+mRqGRSXaO7iLlqHNEw 80GWu3maNLw17dH3wVxHZ1QIwWif9e7Pr+rOAHP9EpieLkiWBK0vu8x6CWb7iZrx cfzW/BlSyQUbS8fn4VBt =+jzW -----END PGP SIGNATURE----- --Ls2Gy6y7jbHLe9Od--