Re: [Qemu-ppc] [PATCH v2 1/2] spapr: Add support for hwrng when available

[Alexander Graf]

> Am 10.09.2015 um 14:03 schrieb Thomas Huth <address@hidden>:
> 
>> 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.


Alex