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

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2462 bytes --]

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:
> 
> diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c
> index c51eb8e..8e7f6b4 100644
> --- a/hw/ppc/spapr_vio.c
> +++ b/hw/ppc/spapr_vio.c
> @@ -99,6 +99,14 @@ static int vio_make_devnode(VIOsPAPRDevice *dev,
>      int vdevice_off, node_off, ret;
>      char *dt_name;
> 
> +    if (!pc->dt_name) {
> +        ret = 0;
> +        if (pc->devnode) {
> +            ret = (pc->devnode)(dev, fdt, -1);
> +        }
> +        return ret;
> +    }
> +
>      vdevice_off = fdt_path_offset(fdt, "/vdevice");
>      if (vdevice_off < 0) {
>          return vdevice_off;
> 
> 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).

-- 
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

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]