On Fri, Sep 11, 2015 at 09:30:28AM +0200, Thomas Huth wrote: > 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 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?) > > > > 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.
Not arguing with that.
In this case it happened because I just copied the setup code from
mc146818rtc which also doesn't set a description.
> >> 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.
>
> 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?) :
Tell me about it. The fact that there are apparently a whole bunch of
conventions about how QOM things should be done that are neither
obvious nor document is starting to really irritate me.
> Instead of using a bus, simply set parent = 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...
From agraf's comment, this seems like the way to go.
I'm still pretty confused about where such a device sits in the
composition tree. I had thought that SysBus was the root of the qdev
tree, but apparently not.
--
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
pgpqs0sR2LEVy.pgp
Description: PGP signature
