On Thu, 2021-05-27 at 13:50 +0100, Mark Brown wrote:

On Thu, May 27, 2021 at 09:54:03AM +1000, Benjamin Herrenschmidt
wrote:

Hi Mark !

Like the comments say the arm64 RNDR register is not intended to
be directly providing entropy but rather returning the output of
a DRBG which is seeded from an actual entropy source whereas the
SMCCC call is intended to return something from an actual entropy
source directly.

Right, I was thinking about using RNDRSS instead.

  The thinking was that the latter is more in
line with what the callers are expecting for the _seed variants,
it's kind of unclear what the performance/quality tradeoff is
supposed to be though.

See the thread at

  

https://lore.kernel.org/r/CAKv+Gu8y4zwpesytU7vffSCuq8YAjWcHwFHwa_LhTW_cLiSfQw@mail.gmail.com (local)

for discussion of the use of RNDR rather than RNDRRS - the main
concern was the potential for issues with excessive use of
entropy especially on bigger systems, possibly amplified if
there's lots of VMs, though in the meantime the random.c code got
updated to read seeds a lot less often (390596c9959c2a4f5b4
"random: avoid arch_get_random_seed_long() when collecting IRQ
randomness") so the considerations have changed a bit since the
original discussion and we could probably already revisit that
one already.

Right. It's also a lot of assumptions without data on what the actual
implementations can do but I suppose there was no much choice here :-)

quoted

Also, why not implement arch_get_random_* using RNDR rather than
forcing these through the kernel CNRG ?

That was the same consideration with potential issues due to
excessive usage, it was dropped at the same time as switching to
RNDR for the _seed variants.  

This was all before I started looking at the series but my
understanding is that the general idea was to go for a
conservative implementation initially and then reevaluate once
we've got actual systems and can measure how things perform in
practice.

In practice most of the non-seed arch_get_random usage is either
a fallback if there's no seed variant or mixed in with the CRNG
output anyway.

Right but I still don't believe the end result makes a whole lot of
sense. In absence of SMCCC we end up using RNDR as a seed which hits
the "not a great match" comment. Not a huge deal I suppose, for our
(EC2) case, we could just not implement the SMCCC call, and let it use
RNDR, it's still going to be better than no HW random source, I just
don't like those firmware calls much.

The arch_get_random_ interfaces already provide a return code
which the callers can handle as needed, they can do something
appropriate for the scenario rather than the architecture code
having to pick.  Sometimes that's to retry later (random.c does
this when seeding for example), sometimes that's to just carry on
with whatever other entropy they've already got.

Ok. It's unfortunate that the ISA is so vague on the circumstances
where the instructions are allowed to fail... it says "a reasonable
amount of time", it may as well have said a "random amount of time" for
the usefulness of it ;-)

The implementation I'm aware of will fail extremely rarely when the HW
detects an issues that requires corrective action, but I could imagine
some implemetations just failing when there's no entropy at hand (esp.
with RNDRSS).

As long as the callers don't give up permanently, that's fine. I was
just a bit concerned by cnrg_init_try_arch{_early}. It would be
preferable for these to "try harder".

Cheers,
Ben.



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel