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

[David Gibson]

On Fri, Aug 12, 2016 at 12:13:49PM +0530, Nikunj A Dadhania wrote:
> David Gibson <address@hidden> writes:
> 
> > [ Unknown signature status ]
> > On Tue, Aug 09, 2016 at 02:47:46PM +0530, Nikunj A Dadhania wrote:
> >> Nikunj A Dadhania <address@hidden> writes:
> >> 
> >> > David Gibson <address@hidden> 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.

Hrm.   From a helper, there's nothing inherently wrong with reading
from /dev/random, AFAIK.  However, it may not work on non-Linux hosts,
and doesn't let you connect it to urandom instead which probably make
it unsuitable to be merged.

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

signature.asc
Description: PGP signature