Re: [Qemu-devel] [Qemu-ppc] [PATCH 2/6] target-ppc: Implement darn instruction

[Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>]

Thomas Huth <thuth@redhat.com> writes:

> On 12.08.2016 08:43, Nikunj A Dadhania wrote:
>> David Gibson <david@gibson.dropbear.id.au> writes:
>> 
>>> [ Unknown signature status ]
>>> On Tue, Aug 09, 2016 at 02:47:46PM +0530, Nikunj A Dadhania wrote:
>>>> Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> writes:
>>>>
>>>>> David Gibson <david@gibson.dropbear.id.au> writes:
>>>>>
>>>>>> [ Unknown signature status ]
>>>>>> On Mon, Aug 08, 2016 at 07:33:37AM +1000, Benjamin Herrenschmidt wrote:
>>>>>>> On Sun, 2016-08-07 at 23:06 +0530, Nikunj A Dadhania wrote:
>>>>>>>> +target_ulong helper_darn(uint32_t l)
>>>>>>>> +{
>>>>>>>> +    target_ulong r = UINT64_MAX;
>>>>>>>> +
>>>>>>>> +    if (l <= 2) {
>>>>>>>> +        do {
>>>>>>>> +            r = random() * random();
>>>>>>>> +            r &= l ? UINT64_MAX : UINT32_MAX;
>>>>>>>> +        } while (r == UINT64_MAX);
>>>>>>>> +    }
>>>>>>>> +
>>>>>>>> +    return r;
>>>>>>>> +}
>>>>>>>>  #endif
>>>>>>>
>>>>>>> Isn't this a bit week ? Look at the implementation of H_RANDOM...
>>>>>>
>>>>>> Indeed, you should be using the rng backend that H_RANDOM, virtio-rng
>>>>>> and the Intel random number instruction all use.
>>>>
>>>> Can you point me to the intel instruction, I couldn't get rdrand
>>>> implementation.
>>>
>>> Ah.. turns out no.  I'd assumed it was there and used the same backend
>>> as virtio-rng and H_RANDOM, but I hadn't actually looked at the code,
>>> and now that I'm trying I can't see it either.
>>>
>>>>
>>>>> I was looking at implementing this, AFAIU, I have to get a new RNG
>>>>> object in the initialization routine. We would need an instance of this
>>>>> per machine. So for pseries I can add in ppc_spapr_init(). I am not sure
>>>>> in case of linux-user where should this be initialized.
>>>>>
>>>>> One other place was init_proc_POWER9(), but that will be per cpu and
>>>>> member of CPUPPCState structure. Advantage is it will work for system
>>>>> emulation and linux-user both and we would not need a lock.
>>>>
>>>> More issues here. Random backend is not compiled for linux-user, adding
>>>> that wasn't difficult, but then rng_backend_request_entropy() uses
>>>> qemu_set_fd_handler() which is a stub for linux-user
>>>> (stubs/set-fd-handler.c)7
>>>
>>>
>>> Ah.. yeah, not sure how we'll need to handle that.
>> 
>> I have sent updated patch, reading from /dev/random. Not sure if that is
>> allowed in tcg. Works fine though.
>
> You can not rely on /dev/random for this job, since it might block. So
> your guest would stop executing when there is not enough random data
> available in the host, and I think that's quite a bad behavior...

Hmm.. rng-random does use this, but it might have way to time out probably:

backends/rng-random.c:    s->filename = g_strdup("/dev/random");

Regards
Nikunj