linux-kernel - Re: [PATCH v2 2/2] HWRNG: thunderx: Add Cavium HWRNG driver for ThunderX SoC.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <57BE28AD.5080607@gmail.com>
Date:   Wed, 24 Aug 2016 16:07:25 -0700
From:   David Daney <ddaney.cavm@...il.com>
To:     Corentin LABBE <clabbe.montjoie@...il.com>
CC:     Omer Khaliq <okhaliq@...iumnetworks.com>,
        linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org,
        linux-crypto@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        bhelgaas@...gle.com, mpm@...enic.com, herbert@...dor.apana.org.au,
        Ananth.Jasty@...ium.com, David.Daney@...ium.com
Subject: Re: [PATCH v2 2/2] HWRNG: thunderx: Add Cavium HWRNG driver for ThunderX
 SoC.

On 08/23/2016 10:46 PM, Corentin LABBE wrote:
> Hello
>
>> +/* Read data from the RNG unit */
>> +static int cavium_rng_read(struct hwrng *rng, void *dat, size_t max, bool wait)
>> +{
>> +	struct cavium_rng *p = container_of(rng, struct cavium_rng, ops);
>> +	unsigned int size = max;
>> +
>> +	while (size >= 8) {
>> +		*((u64 *)dat) = readq(p->result);
>> +		size -= 8;
>> +		dat += 8;
>> +	}
>
> I think you could use readsq()
> This will increase throughput

If you look at the implementation of readsq(), you will see that it is a 
similar loop.  Since the overhead is primarily I/O latency from the RNG 
hardware, the throughput cannot really be changed with micro 
optimizations to this simple loop.

Also, on big-endian kernels, it appears that a loop of readq() and 
readsq() will give different results as readq will byte swap the result 
and readsq does not.  Since this is a RNG, the byte swapping is not 
important, but it is a difference.

Because of this, I think it should be acceptable to stick with the loop 
we currently have.

If the hwrng maintainers want to change the loop, to a readsq(), we 
might investigate this more.

Thanks,
David Daney