[Qemu-devel] [PATCH] spapr: add a default rng device

[Qemu-devel] [PATCH] spapr: add a default rng device

[Greg Kurz]

A recent patch by Thomas Huth brought a new spapr-rng pseudo-device to
provide high-quality random numbers to guests. The device may either be
backed by a "RngBackend" or the in-kernel implementation of the H_RANDOM
hypercall.

Since modern POWER8 based servers always provide a hardware rng, it makes
sense to create a spapr-rng device with use-kvm=true by default when it
is available.

Of course we want the user to have full control on how the rng is handled.
The default device WILL NOT be created in the following cases:
- the -nodefaults option was passed
- a spapr-rng device was already passed on the command line

The default device is created at reset time to ensure devices specified on
the command line have been created.

Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
---
 hw/ppc/spapr.c       |   17 +++++++++++++++++
 hw/ppc/spapr_rng.c   |    2 +-
 target-ppc/kvm.c     |    9 +++++----
 target-ppc/kvm_ppc.h |    6 ++++++
 4 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 7f4f196e53e5..ee048ecffd0c 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1059,6 +1059,14 @@ static int spapr_check_htab_fd(sPAPRMachineState *spapr)
     return rc;
 }
 
+static void spapr_rng_create(void)
+{
+    Object *rng = object_new(TYPE_SPAPR_RNG);
+
+    object_property_set_bool(rng, true, "use-kvm", &error_abort);
+    object_property_set_bool(rng, true, "realized", &error_abort);
+}
+
 static void ppc_spapr_reset(void)
 {
     sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
@@ -1082,6 +1090,15 @@ static void ppc_spapr_reset(void)
     spapr->rtas_addr = rtas_limit - RTAS_MAX_SIZE;
     spapr->fdt_addr = spapr->rtas_addr - FDT_MAX_SIZE;
 
+    /* Create a rng device if the user did not provide it already and
+     * KVM has hwrng support.
+     */
+    if (defaults_enabled() &&
+        kvmppc_hwrng_present() &&
+        !object_resolve_path_type("", TYPE_SPAPR_RNG, NULL)) {
+        spapr_rng_create();
+    }
+
     /* Load the fdt */
     spapr_finalize_fdt(spapr, spapr->fdt_addr, spapr->rtas_addr,
                        spapr->rtas_size);
diff --git a/hw/ppc/spapr_rng.c b/hw/ppc/spapr_rng.c
index ed43d5e04221..ee5af302bd4d 100644
--- a/hw/ppc/spapr_rng.c
+++ b/hw/ppc/spapr_rng.c
@@ -114,7 +114,7 @@ static void spapr_rng_realize(DeviceState *dev, Error **errp)
     sPAPRRngState *rngstate = SPAPR_RNG(dev);
 
     if (rngstate->use_kvm) {
-        if (kvmppc_enable_hwrng() == 0) {
+        if (kvmppc_hwrng_present() && kvmppc_enable_hwrng() == 0) {
             return;
         }
         /*
diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index e641680fb146..084bb034f1fd 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -2490,11 +2490,12 @@ int kvm_arch_msi_data_to_gsi(uint32_t data)
     return data & 0xffff;
 }
 
-int kvmppc_enable_hwrng(void)
+bool kvmppc_hwrng_present(void)
 {
-    if (!kvm_enabled() || !kvm_check_extension(kvm_state, KVM_CAP_PPC_HWRNG)) {
-        return -1;
-    }
+    return kvm_enabled() && kvm_check_extension(kvm_state, KVM_CAP_PPC_HWRNG);
+}
 
+int kvmppc_enable_hwrng(void)
+{
     return kvmppc_enable_hcall(kvm_state, H_RANDOM);
 }
diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h
index 470f6d62f7bb..a76338c9aa16 100644
--- a/target-ppc/kvm_ppc.h
+++ b/target-ppc/kvm_ppc.h
@@ -54,6 +54,7 @@ void kvmppc_hash64_free_pteg(uint64_t token);
 void kvmppc_hash64_write_pte(CPUPPCState *env, target_ulong pte_index,
                              target_ulong pte0, target_ulong pte1);
 bool kvmppc_has_cap_fixup_hcalls(void);
+bool kvmppc_hwrng_present(void);
 int kvmppc_enable_hwrng(void);
 
 #else
@@ -252,6 +253,11 @@ static inline bool kvmppc_has_cap_fixup_hcalls(void)
     abort();
 }
 
+static inline bool kvmppc_hwrng_present(void)
+{
+    return false;
+}
+
 static inline int kvmppc_enable_hwrng(void)
 {
     return -1;

Re: [Qemu-devel] [PATCH] spapr: add a default rng device

[Greg Kurz]

Cc'ing qemu-ppc@

On Mon, 28 Sep 2015 12:13:47 +0200
Greg Kurz <gkurz@linux.vnet.ibm.com> wrote:
> A recent patch by Thomas Huth brought a new spapr-rng pseudo-device to
> provide high-quality random numbers to guests. The device may either be
> backed by a "RngBackend" or the in-kernel implementation of the H_RANDOM
> hypercall.
> 
> Since modern POWER8 based servers always provide a hardware rng, it makes
> sense to create a spapr-rng device with use-kvm=true by default when it
> is available.
> 
> Of course we want the user to have full control on how the rng is handled.
> The default device WILL NOT be created in the following cases:
> - the -nodefaults option was passed
> - a spapr-rng device was already passed on the command line
> 
> The default device is created at reset time to ensure devices specified on
> the command line have been created.
> 
> Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
> ---
>  hw/ppc/spapr.c       |   17 +++++++++++++++++
>  hw/ppc/spapr_rng.c   |    2 +-
>  target-ppc/kvm.c     |    9 +++++----
>  target-ppc/kvm_ppc.h |    6 ++++++
>  4 files changed, 29 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 7f4f196e53e5..ee048ecffd0c 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1059,6 +1059,14 @@ static int spapr_check_htab_fd(sPAPRMachineState *spapr)
>      return rc;
>  }
> 
> +static void spapr_rng_create(void)
> +{
> +    Object *rng = object_new(TYPE_SPAPR_RNG);
> +
> +    object_property_set_bool(rng, true, "use-kvm", &error_abort);
> +    object_property_set_bool(rng, true, "realized", &error_abort);
> +}
> +
>  static void ppc_spapr_reset(void)
>  {
>      sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
> @@ -1082,6 +1090,15 @@ static void ppc_spapr_reset(void)
>      spapr->rtas_addr = rtas_limit - RTAS_MAX_SIZE;
>      spapr->fdt_addr = spapr->rtas_addr - FDT_MAX_SIZE;
> 
> +    /* Create a rng device if the user did not provide it already and
> +     * KVM has hwrng support.
> +     */
> +    if (defaults_enabled() &&
> +        kvmppc_hwrng_present() &&
> +        !object_resolve_path_type("", TYPE_SPAPR_RNG, NULL)) {
> +        spapr_rng_create();
> +    }
> +
>      /* Load the fdt */
>      spapr_finalize_fdt(spapr, spapr->fdt_addr, spapr->rtas_addr,
>                         spapr->rtas_size);
> diff --git a/hw/ppc/spapr_rng.c b/hw/ppc/spapr_rng.c
> index ed43d5e04221..ee5af302bd4d 100644
> --- a/hw/ppc/spapr_rng.c
> +++ b/hw/ppc/spapr_rng.c
> @@ -114,7 +114,7 @@ static void spapr_rng_realize(DeviceState *dev, Error **errp)
>      sPAPRRngState *rngstate = SPAPR_RNG(dev);
> 
>      if (rngstate->use_kvm) {
> -        if (kvmppc_enable_hwrng() == 0) {
> +        if (kvmppc_hwrng_present() && kvmppc_enable_hwrng() == 0) {
>              return;
>          }
>          /*
> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> index e641680fb146..084bb034f1fd 100644
> --- a/target-ppc/kvm.c
> +++ b/target-ppc/kvm.c
> @@ -2490,11 +2490,12 @@ int kvm_arch_msi_data_to_gsi(uint32_t data)
>      return data & 0xffff;
>  }
> 
> -int kvmppc_enable_hwrng(void)
> +bool kvmppc_hwrng_present(void)
>  {
> -    if (!kvm_enabled() || !kvm_check_extension(kvm_state, KVM_CAP_PPC_HWRNG)) {
> -        return -1;
> -    }
> +    return kvm_enabled() && kvm_check_extension(kvm_state, KVM_CAP_PPC_HWRNG);
> +}
> 
> +int kvmppc_enable_hwrng(void)
> +{
>      return kvmppc_enable_hcall(kvm_state, H_RANDOM);
>  }
> diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h
> index 470f6d62f7bb..a76338c9aa16 100644
> --- a/target-ppc/kvm_ppc.h
> +++ b/target-ppc/kvm_ppc.h
> @@ -54,6 +54,7 @@ void kvmppc_hash64_free_pteg(uint64_t token);
>  void kvmppc_hash64_write_pte(CPUPPCState *env, target_ulong pte_index,
>                               target_ulong pte0, target_ulong pte1);
>  bool kvmppc_has_cap_fixup_hcalls(void);
> +bool kvmppc_hwrng_present(void);
>  int kvmppc_enable_hwrng(void);
> 
>  #else
> @@ -252,6 +253,11 @@ static inline bool kvmppc_has_cap_fixup_hcalls(void)
>      abort();
>  }
> 
> +static inline bool kvmppc_hwrng_present(void)
> +{
> +    return false;
> +}
> +
>  static inline int kvmppc_enable_hwrng(void)
>  {
>      return -1;
> 
> 

Re: [Qemu-devel] [PATCH] spapr: add a default rng device

[David Gibson]

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

On Mon, Sep 28, 2015 at 12:13:47PM +0200, Greg Kurz wrote:
> A recent patch by Thomas Huth brought a new spapr-rng pseudo-device to
> provide high-quality random numbers to guests. The device may either be
> backed by a "RngBackend" or the in-kernel implementation of the H_RANDOM
> hypercall.
> 
> Since modern POWER8 based servers always provide a hardware rng, it makes
> sense to create a spapr-rng device with use-kvm=true by default when it
> is available.
> 
> Of course we want the user to have full control on how the rng is handled.
> The default device WILL NOT be created in the following cases:
> - the -nodefaults option was passed
> - a spapr-rng device was already passed on the command line
> 
> The default device is created at reset time to ensure devices specified on
> the command line have been created.
> 
> Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>

So, I think the concept is ok, but..

> ---
>  hw/ppc/spapr.c       |   17 +++++++++++++++++
>  hw/ppc/spapr_rng.c   |    2 +-
>  target-ppc/kvm.c     |    9 +++++----
>  target-ppc/kvm_ppc.h |    6 ++++++
>  4 files changed, 29 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 7f4f196e53e5..ee048ecffd0c 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1059,6 +1059,14 @@ static int spapr_check_htab_fd(sPAPRMachineState *spapr)
>      return rc;
>  }
>  
> +static void spapr_rng_create(void)
> +{
> +    Object *rng = object_new(TYPE_SPAPR_RNG);
> +
> +    object_property_set_bool(rng, true, "use-kvm", &error_abort);
> +    object_property_set_bool(rng, true, "realized", &error_abort);
> +}
> +
>  static void ppc_spapr_reset(void)
>  {
>      sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
> @@ -1082,6 +1090,15 @@ static void ppc_spapr_reset(void)
>      spapr->rtas_addr = rtas_limit - RTAS_MAX_SIZE;
>      spapr->fdt_addr = spapr->rtas_addr - FDT_MAX_SIZE;
>  
> +    /* Create a rng device if the user did not provide it already and
> +     * KVM has hwrng support.
> +     */
> +    if (defaults_enabled() &&
> +        kvmppc_hwrng_present() &&
> +        !object_resolve_path_type("", TYPE_SPAPR_RNG, NULL)) {
> +        spapr_rng_create();
> +    }
> +

Constructing the RNG at reset time is just wrong.  Using
defaults_enabled() is ugly at the best of times, using it at reset,
after construction of the qom tree is generally complete, is just
hideous.

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

Re: [Qemu-devel] [PATCH] spapr: add a default rng device

[Greg Kurz]

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

On Tue, 29 Sep 2015 15:01:09 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Mon, Sep 28, 2015 at 12:13:47PM +0200, Greg Kurz wrote:
> > A recent patch by Thomas Huth brought a new spapr-rng pseudo-device to
> > provide high-quality random numbers to guests. The device may either be
> > backed by a "RngBackend" or the in-kernel implementation of the H_RANDOM
> > hypercall.
> > 
> > Since modern POWER8 based servers always provide a hardware rng, it makes
> > sense to create a spapr-rng device with use-kvm=true by default when it
> > is available.
> > 
> > Of course we want the user to have full control on how the rng is handled.
> > The default device WILL NOT be created in the following cases:
> > - the -nodefaults option was passed
> > - a spapr-rng device was already passed on the command line
> > 
> > The default device is created at reset time to ensure devices specified on
> > the command line have been created.
> > 
> > Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
> 
> So, I think the concept is ok, but..
> 

Just to be sure about the concept.

The goal is to free users from having to explicitely pass

	-device spapr-rng,use-kvm=true

... when ALL the following conditions are met:

1) KVM is used and advertises KVM_CAP_PPC_HWRNG
2) -nodefaults HAS NOT been passed on the cmdline
3) -device spapr-rng HAS NOT been passed on the cmdline

> > ---
> >  hw/ppc/spapr.c       |   17 +++++++++++++++++
> >  hw/ppc/spapr_rng.c   |    2 +-
> >  target-ppc/kvm.c     |    9 +++++----
> >  target-ppc/kvm_ppc.h |    6 ++++++
> >  4 files changed, 29 insertions(+), 5 deletions(-)
> > 
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 7f4f196e53e5..ee048ecffd0c 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -1059,6 +1059,14 @@ static int spapr_check_htab_fd(sPAPRMachineState *spapr)
> >      return rc;
> >  }
> >  
> > +static void spapr_rng_create(void)
> > +{
> > +    Object *rng = object_new(TYPE_SPAPR_RNG);
> > +
> > +    object_property_set_bool(rng, true, "use-kvm", &error_abort);
> > +    object_property_set_bool(rng, true, "realized", &error_abort);
> > +}
> > +
> >  static void ppc_spapr_reset(void)
> >  {
> >      sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
> > @@ -1082,6 +1090,15 @@ static void ppc_spapr_reset(void)
> >      spapr->rtas_addr = rtas_limit - RTAS_MAX_SIZE;
> >      spapr->fdt_addr = spapr->rtas_addr - FDT_MAX_SIZE;
> >  
> > +    /* Create a rng device if the user did not provide it already and
> > +     * KVM has hwrng support.
> > +     */
> > +    if (defaults_enabled() &&
> > +        kvmppc_hwrng_present() &&
> > +        !object_resolve_path_type("", TYPE_SPAPR_RNG, NULL)) {
> > +        spapr_rng_create();
> > +    }
> > +
> 
> Constructing the RNG at reset time is just wrong.  Using
> defaults_enabled() is ugly at the best of times, using it at reset,
> after construction of the qom tree is generally complete, is just
> hideous.
> 

Yeah I ended up with this hack because I could not figure out how
to give priority to a spapr-rng device specified on the cmdline
over the automatic one... poor QOM skills :\

If you have a suggestion to handle this case in a more appropriate way,
and it is worth the pain compared to the gain, please advice.

Thanks.

--
Greg

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [Qemu-devel] [PATCH] spapr: add a default rng device

[Thomas Huth]

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

On 30/09/15 10:33, Greg Kurz wrote:
> On Tue, 29 Sep 2015 15:01:09 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
>> On Mon, Sep 28, 2015 at 12:13:47PM +0200, Greg Kurz wrote:
>>> A recent patch by Thomas Huth brought a new spapr-rng pseudo-device to
>>> provide high-quality random numbers to guests. The device may either be
>>> backed by a "RngBackend" or the in-kernel implementation of the H_RANDOM
>>> hypercall.
>>>
>>> Since modern POWER8 based servers always provide a hardware rng, it makes
>>> sense to create a spapr-rng device with use-kvm=true by default when it
>>> is available.
>>>
>>> Of course we want the user to have full control on how the rng is handled.
>>> The default device WILL NOT be created in the following cases:
>>> - the -nodefaults option was passed
>>> - a spapr-rng device was already passed on the command line
>>>
>>> The default device is created at reset time to ensure devices specified on
>>> the command line have been created.
>>>
>>> Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
>>
>> So, I think the concept is ok, but..
>>
> 
> Just to be sure about the concept.
> 
> The goal is to free users from having to explicitely pass
> 
> 	-device spapr-rng,use-kvm=true
> 
> ... when ALL the following conditions are met:
> 
> 1) KVM is used and advertises KVM_CAP_PPC_HWRNG
> 2) -nodefaults HAS NOT been passed on the cmdline
> 3) -device spapr-rng HAS NOT been passed on the cmdline
> 
>>> ---
>>>  hw/ppc/spapr.c       |   17 +++++++++++++++++
>>>  hw/ppc/spapr_rng.c   |    2 +-
>>>  target-ppc/kvm.c     |    9 +++++----
>>>  target-ppc/kvm_ppc.h |    6 ++++++
>>>  4 files changed, 29 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>>> index 7f4f196e53e5..ee048ecffd0c 100644
>>> --- a/hw/ppc/spapr.c
>>> +++ b/hw/ppc/spapr.c
>>> @@ -1059,6 +1059,14 @@ static int spapr_check_htab_fd(sPAPRMachineState *spapr)
>>>      return rc;
>>>  }
>>>  
>>> +static void spapr_rng_create(void)
>>> +{
>>> +    Object *rng = object_new(TYPE_SPAPR_RNG);
>>> +
>>> +    object_property_set_bool(rng, true, "use-kvm", &error_abort);
>>> +    object_property_set_bool(rng, true, "realized", &error_abort);
>>> +}
>>> +
>>>  static void ppc_spapr_reset(void)
>>>  {
>>>      sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
>>> @@ -1082,6 +1090,15 @@ static void ppc_spapr_reset(void)
>>>      spapr->rtas_addr = rtas_limit - RTAS_MAX_SIZE;
>>>      spapr->fdt_addr = spapr->rtas_addr - FDT_MAX_SIZE;
>>>  
>>> +    /* Create a rng device if the user did not provide it already and
>>> +     * KVM has hwrng support.
>>> +     */
>>> +    if (defaults_enabled() &&
>>> +        kvmppc_hwrng_present() &&
>>> +        !object_resolve_path_type("", TYPE_SPAPR_RNG, NULL)) {
>>> +        spapr_rng_create();
>>> +    }
>>> +
>>
>> Constructing the RNG at reset time is just wrong.  Using
>> defaults_enabled() is ugly at the best of times, using it at reset,
>> after construction of the qom tree is generally complete, is just
>> hideous.
>>
> 
> Yeah I ended up with this hack because I could not figure out how
> to give priority to a spapr-rng device specified on the cmdline
> over the automatic one... poor QOM skills :\
> 
> If you have a suggestion to handle this case in a more appropriate way,
> and it is worth the pain compared to the gain, please advice.

Not sure whether this might be an acceptable solution, but maybe you
could use qemu_opts_foreach(qemu_find_opts("device"), ...) to check
whether a "spapr-rng" device has been specified at the command line?

 Thomas



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [Qemu-devel] [PATCH] spapr: add a default rng device

[Greg Kurz]

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

On Wed, 30 Sep 2015 11:10:52 +0200
Thomas Huth <thuth@redhat.com> wrote:

> On 30/09/15 10:33, Greg Kurz wrote:
> > On Tue, 29 Sep 2015 15:01:09 +1000
> > David Gibson <david@gibson.dropbear.id.au> wrote:
> > 
> >> On Mon, Sep 28, 2015 at 12:13:47PM +0200, Greg Kurz wrote:
> >>> A recent patch by Thomas Huth brought a new spapr-rng pseudo-device to
> >>> provide high-quality random numbers to guests. The device may either be
> >>> backed by a "RngBackend" or the in-kernel implementation of the H_RANDOM
> >>> hypercall.
> >>>
> >>> Since modern POWER8 based servers always provide a hardware rng, it makes
> >>> sense to create a spapr-rng device with use-kvm=true by default when it
> >>> is available.
> >>>
> >>> Of course we want the user to have full control on how the rng is handled.
> >>> The default device WILL NOT be created in the following cases:
> >>> - the -nodefaults option was passed
> >>> - a spapr-rng device was already passed on the command line
> >>>
> >>> The default device is created at reset time to ensure devices specified on
> >>> the command line have been created.
> >>>
> >>> Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
> >>
> >> So, I think the concept is ok, but..
> >>
> > 
> > Just to be sure about the concept.
> > 
> > The goal is to free users from having to explicitely pass
> > 
> > 	-device spapr-rng,use-kvm=true
> > 
> > ... when ALL the following conditions are met:
> > 
> > 1) KVM is used and advertises KVM_CAP_PPC_HWRNG
> > 2) -nodefaults HAS NOT been passed on the cmdline
> > 3) -device spapr-rng HAS NOT been passed on the cmdline
> > 
> >>> ---
> >>>  hw/ppc/spapr.c       |   17 +++++++++++++++++
> >>>  hw/ppc/spapr_rng.c   |    2 +-
> >>>  target-ppc/kvm.c     |    9 +++++----
> >>>  target-ppc/kvm_ppc.h |    6 ++++++
> >>>  4 files changed, 29 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> >>> index 7f4f196e53e5..ee048ecffd0c 100644
> >>> --- a/hw/ppc/spapr.c
> >>> +++ b/hw/ppc/spapr.c
> >>> @@ -1059,6 +1059,14 @@ static int spapr_check_htab_fd(sPAPRMachineState *spapr)
> >>>      return rc;
> >>>  }
> >>>  
> >>> +static void spapr_rng_create(void)
> >>> +{
> >>> +    Object *rng = object_new(TYPE_SPAPR_RNG);
> >>> +
> >>> +    object_property_set_bool(rng, true, "use-kvm", &error_abort);
> >>> +    object_property_set_bool(rng, true, "realized", &error_abort);
> >>> +}
> >>> +
> >>>  static void ppc_spapr_reset(void)
> >>>  {
> >>>      sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
> >>> @@ -1082,6 +1090,15 @@ static void ppc_spapr_reset(void)
> >>>      spapr->rtas_addr = rtas_limit - RTAS_MAX_SIZE;
> >>>      spapr->fdt_addr = spapr->rtas_addr - FDT_MAX_SIZE;
> >>>  
> >>> +    /* Create a rng device if the user did not provide it already and
> >>> +     * KVM has hwrng support.
> >>> +     */
> >>> +    if (defaults_enabled() &&
> >>> +        kvmppc_hwrng_present() &&
> >>> +        !object_resolve_path_type("", TYPE_SPAPR_RNG, NULL)) {
> >>> +        spapr_rng_create();
> >>> +    }
> >>> +
> >>
> >> Constructing the RNG at reset time is just wrong.  Using
> >> defaults_enabled() is ugly at the best of times, using it at reset,
> >> after construction of the qom tree is generally complete, is just
> >> hideous.
> >>
> > 
> > Yeah I ended up with this hack because I could not figure out how
> > to give priority to a spapr-rng device specified on the cmdline
> > over the automatic one... poor QOM skills :\
> > 
> > If you have a suggestion to handle this case in a more appropriate way,
> > and it is worth the pain compared to the gain, please advice.
> 
> Not sure whether this might be an acceptable solution, but maybe you
> could use qemu_opts_foreach(qemu_find_opts("device"), ...) to check
> whether a "spapr-rng" device has been specified at the command line?
> 

Yes it would allow, at least, to create the device at init time... then
I don't know if it is good practice, considering that:

$ grep -r qemu_opts_foreach hw/
hw/core/qdev-properties-system.c:    qemu_opts_foreach(qemu_find_opts("global"),
$

Cheers.

--
Greg

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]