[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] spapr: add a default rng device
From: |
Thomas Huth |
Subject: |
Re: [Qemu-devel] [PATCH] spapr: add a default rng device |
Date: |
Wed, 30 Sep 2015 11:10:52 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 |
On 30/09/15 10:33, Greg Kurz wrote:
> On Tue, 29 Sep 2015 15:01:09 +1000
> David Gibson <address@hidden> 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 <address@hidden>
>>
>> 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
signature.asc
Description: OpenPGP digital signature