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 <th...@redhat.com>: >>> >>>> 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. 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. Alex
