On Tue, 29 Sep 2015 15:01:09 +1000 David Gibson <da...@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 <gk...@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
pgp7IIPM0K3WF.pgp
Description: OpenPGP digital signature
