From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51857) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZaCaq-0005ie-3G for qemu-devel@nongnu.org; Thu, 10 Sep 2015 20:53:33 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZaCam-0005rB-9y for qemu-devel@nongnu.org; Thu, 10 Sep 2015 20:53:32 -0400 Date: Fri, 11 Sep 2015 10:45:28 +1000 From: David Gibson Message-ID: <20150911004527.GF11781@voom> 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> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="R6sEYoIZpp9JErk7" Content-Disposition: inline In-Reply-To: <55F1719B.5070208@redhat.com> 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: Thomas Huth Cc: armbru@redhat.com, qemu-devel@nongnu.org, michael@ellerman.id.au, qemu-ppc@nongnu.org, amit.shah@redhat.com, Sam Bobroff --R6sEYoIZpp9JErk7 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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 machine > >>>> 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 class > >>> like it is done for spapr-rtc, since the user apparently can not plug > >>> device to this bus on machine spapr (you can also not plug an spapr-r= tc > >>> 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 un= der > >>> /vdevice in the device tree - that's also not what we want, I guess. > >> > >> I did some more tests, and I think I can get this working with one sma= ll > >> modification to spapr_vio.c > ... > >> i.e. when the dt_name has not been set, the device won't be added to t= he > >> /vdevice device tree node. If that's acceptable, I'll continue with th= is > >> approach. > >=20 > > A bit hacky. > >=20 > > 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). >=20 > If anyhow possible, I'd prefere to use "-device" for this instead, because >=20 > 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?) 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. > 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. 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. 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. > 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. I still dislike putting it on the VIO "bus", since PAPR doesn't consider it a VIO device. --=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 --R6sEYoIZpp9JErk7 Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJV8iQnAAoJEGw4ysog2bOSDG4QALQdCpPoRtU9F1lLOKpS8GrW InQk2G9VTDmnRU4ViDR+RUr1IrGi7QIfyPe6irPW5rkfR54i9nd5OOllM9OMs9WI gUuejeRqKLDyWhAWBbWHe3aESBIEdStp1ljC8SfFF1w5J+BrV14zZgYVcSxNNHyv SCM7ffhUjPbxizmqFUiuQmmyC7TexxYkF1PO4BiNPtxU+FNGs8rdmLPeT6Cscsvv reFduh/jlcrzLctt92xxQf87mFyTFaXWCqlJXGM4xPi1D+Q4/YQRf5QrNuF5UbJB vg4ce0hSTFe/Fuq7p3Y9933SvhfihRYvu1tLsmxWOtJ3vzQ5jpx4QfXVGhfGoDDB wHeSjAUGfkkPbpU2SHbSMsI8m53wwywgHpTXk7y6k/YChwIB+k0EI4qtaCMG9CPM PmyERpz9FlGuwO21bcg2hBir2tvDvTngcMSesE6biNh9GK3MerdZ0TKPM4BfCsjI YaPzmigEHblZZcxVvJm3qk8/+IhwAS1rsDUMh8yo5TlUcERQM5dRsuGx3ibEpcnI gZBq5/7yjSJ3wWvUYGjHE2DuXk/SOORIMi/g/lbexbXTtQD/5FaK/zD0FpaUrQL3 uTnQYOAHrVfYSMzre1ldHXwGCXGycEJp49ZWphaqSJemf1adZfB7PguRcIw3+I+s hi7aOAhvscLDZuRqKyQY =0q/+ -----END PGP SIGNATURE----- --R6sEYoIZpp9JErk7--