From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46772) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZbOJq-0004uy-PY for qemu-devel@nongnu.org; Mon, 14 Sep 2015 03:36:55 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZbOJn-0001fT-IS for qemu-devel@nongnu.org; Mon, 14 Sep 2015 03:36:54 -0400 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> <20150914022705.GF2547@voom.fritz.box> From: Alexander Graf Message-ID: <55F67910.5040207@suse.de> Date: Mon, 14 Sep 2015 09:36:48 +0200 MIME-Version: 1.0 In-Reply-To: <20150914022705.GF2547@voom.fritz.box> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit 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: Thomas Huth , "armbru@redhat.com" , "qemu-devel@nongnu.org" , "michael@ellerman.id.au" , "qemu-ppc@nongnu.org" , "amit.shah@redhat.com" , Sam Bobroff On 14.09.15 04:27, David Gibson wrote: > On Fri, Sep 11, 2015 at 11:43:02AM +0200, Alexander Graf wrote: >> >> >> 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 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=rng0,usekvm=0 >>>>>>>> >>>>>>>> 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-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. >>>>>>> >>>>>>> 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 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 (or >>>>>> -set, but -global is easier). >>>>> >>>>> If anyhow possible, I'd prefere to use "-device" for this instead, because >>>>> >>>>> 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. >>> >>> Ok.. why? >>> >>> It's a system (pseudo-)device that doesn't have any common bus >>> infrastructure with anything else. Isn't that what SysBus is for? >> >> 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? You could ask the same about any bus really. It's more or less common sense / collective knowledge / call it what you want. Just check out the sysbus code files and you'll see that 90% of them are about handling mmio / pio and irqs. Do you need that logic? No? Then sysbus is not for you :). > >> 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? qdev is legacy, qom is new :). In qdev sysbus was the root bus, in qom it's not. For details on what exactly is the root for qom, please just poke Andreas - I keep having a hard time to wrap my head around the qom topology. I'm not even sure it has a root in the traditional sense. Alex