Re: [PATCH v2 4/5] arm64: Add support for SMCCC TRNG entropy source
On 05/11/2020 14:38, Mark Rutland wrote: Hi, > On Thu, Nov 05, 2020 at 02:29:49PM +, Mark Brown wrote: >> On Thu, Nov 05, 2020 at 02:03:22PM +, Mark Rutland wrote: >>> On Thu, Nov 05, 2020 at 01:41:42PM +, Mark Brown wrote: >> It isn't obvious to me why we don't fall through to trying the SMCCC TRNG here if for some reason the v8.5-RNG didn't give us something. Definitely an obscure possibility but still... >> >>> I think it's better to assume that if we have a HW RNG and it's not >>> giving us entropy, it's not worthwhile trapping to the host, which might >>> encounter the exact same issue. >> >> There's definitely a good argument for that, but OTOH it's possible the >> SMCCC implementation is doing something else (it'd be an interesting >> implementation decision but...). That said I don't really mind, I think >> my comment was more that if we're doing this the code should be explicit >> about what the intent is since right now it isn't obvious. Either a >> comment or having an explicit "what method are we choosing" thing. >> >>> That said, I'm not sure it's great to plumb this under the >>> arch_get_random*() interfaces, e.g. given this measn that >>> add_interrupt_randomness() will end up trapping to the host all the time >>> when it calls arch_get_random_seed_long(). >> >>> Is there an existing interface for "slow" runtime entropy that we can >>> plumb this into instead? >> >> Yeah, I was wondering about this myself - it seems like a better fit for >> hwrng rather than the arch interfaces but that's not used until >> userspace comes up, the arch stuff is all expected to be quick. I >> suppose we could implement the SMCCC stuff for the early variants of the >> API you added so it gets used for bootstrapping purposes and then we >> rely on userspace keeping things topped up by fetching entropy through >> hwrng or otherwise but that feels confused so I have a hard time getting >> enthusiastic about it. > > I'm perfectly happy for the early functions to call this, or for us to > add something new firmwware_get_random_*() functions that we can call > early (and potentially at runtime, but less often than > arch_get_random_*()). > > I suspect the easy thing to do for now is plumb this into the existing > early arch functions and hwrng. So coming back to this: With Ard's patch to remove arch_get_random from add_interrupt_randomness(), I see this called much less often: basically once at early boot, then 16 longs every 5 minutes or so, from the periodic crng reseed. The only exception would be the KVM code now, so we are at the grace of a guest to not swamp us with seed requests. Alternatively we could remove the direct arch_get_random call from the KVM code, relying on the general kernel pool instead. Is this new situation now good enough to keep the SMCCC calls in this interface here? I have the hwrng driver ready, which could coexist with the arch_random implementation. But if the only purpose of /dev/hwrng is to let rngd feed this entropy back into the kernel, it would be pointless. I found the driver useful to debug and test the firmware implementation and to assess the random number quality (by feeding the raw stream into rngtest or dieharder), but that might not justify a merge. Ard objected against the driver, I guess to keep things simple and architectural. So what is the plan here? Shall I post a v3 with or without the hwrng driver? And do we keep the SMCCC arch_random implementation? Cheers, Andre ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v2 4/5] arm64: Add support for SMCCC TRNG entropy source
On 2020-11-05 14:34, Ard Biesheuvel wrote: On Thu, 5 Nov 2020 at 15:30, Mark Rutland wrote: On Thu, Nov 05, 2020 at 03:04:57PM +0100, Ard Biesheuvel wrote: > On Thu, 5 Nov 2020 at 15:03, Mark Rutland wrote: > > On Thu, Nov 05, 2020 at 01:41:42PM +, Mark Brown wrote: > > > On Thu, Nov 05, 2020 at 12:56:55PM +, Andre Przywara wrote: > > That said, I'm not sure it's great to plumb this under the > > arch_get_random*() interfaces, e.g. given this measn that > > add_interrupt_randomness() will end up trapping to the host all the time > > when it calls arch_get_random_seed_long(). > > As it turns out, add_interrupt_randomness() isn't actually used on ARM. It's certainly called on arm64, per a warning I just hacked in: [1.083802] [ cut here ] [1.084802] add_interrupt_randomness called [1.085685] WARNING: CPU: 1 PID: 0 at drivers/char/random.c:1267 add_interrupt_randomness+0x2e8/0x318 [1.087599] Modules linked in: [1.088258] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 5.10.0-rc2-dirty #13 [1.089672] Hardware name: linux,dummy-virt (DT) [1.090659] pstate: 60400085 (nZCv daIf +PAN -UAO -TCO BTYPE=--) [1.091910] pc : add_interrupt_randomness+0x2e8/0x318 [1.092965] lr : add_interrupt_randomness+0x2e8/0x318 [1.094021] sp : 80001000be80 [1.094732] x29: 80001000be80 x28: 2d0c80209840 [1.095859] x27: 137c3e3a x26: 8000100abdd0 [1.096978] x25: 0035 x24: 67918bda8000 [1.098100] x23: c57c31923fe8 x22: fffedc14 [1.099224] x21: 2d0dbef796a0 x20: c57c331d16a0 [1.100339] x19: c57c33720a48 x18: 0010 [1.101459] x17: x16: 0002 [1.102578] x15: 00e7 x14: 80001000bb20 [1.103706] x13: ffea x12: c57c337b56e8 [1.104821] x11: 0003 x10: c57c3379d6a8 [1.105944] x9 : c57c3379d700 x8 : 00017fe8 [1.107073] x7 : c000efff x6 : 0001 [1.108186] x5 : 00057fa8 x4 : [1.109305] x3 : x2 : c57c337455d0 [1.110428] x1 : db8dc9c2a1e0f600 x0 : [1.111552] Call trace: [1.112083] add_interrupt_randomness+0x2e8/0x318 [1.113074] handle_irq_event_percpu+0x48/0x90 [1.114016] handle_irq_event+0x48/0xf8 [1.114826] handle_fasteoi_irq+0xa4/0x130 [1.115689] generic_handle_irq+0x30/0x48 [1.116528] __handle_domain_irq+0x64/0xc0 [1.117392] gic_handle_irq+0xc0/0x138 [1.118194] el1_irq+0xbc/0x180 [1.118870] arch_cpu_idle+0x20/0x30 [1.119630] default_idle_call+0x8c/0x350 [1.120479] do_idle+0x224/0x298 [1.121163] cpu_startup_entry+0x28/0x70 [1.121994] secondary_start_kernel+0x184/0x198 ... and I couldn't immediately spot why 32-bit arm would be different. Hmm, I actually meant both arm64 and ARM. Marc looked into this at my request a while ago, and I had a look myself as well at the time, and IIRC, we both concluded that we don't hit that code path. Darn. Yes, I remember checking this. Obviously, I need a new pair of glasses... M. -- Jazz is not dead. It just smells funny... ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v2 4/5] arm64: Add support for SMCCC TRNG entropy source
On Thu, Nov 05, 2020 at 03:34:01PM +0100, Ard Biesheuvel wrote: > On Thu, 5 Nov 2020 at 15:30, Mark Rutland wrote: > > On Thu, Nov 05, 2020 at 03:04:57PM +0100, Ard Biesheuvel wrote: > > > On Thu, 5 Nov 2020 at 15:03, Mark Rutland wrote: > > > > > > That said, I'm not sure it's great to plumb this under the > > > > arch_get_random*() interfaces, e.g. given this measn that > > > > add_interrupt_randomness() will end up trapping to the host all the time > > > > when it calls arch_get_random_seed_long(). > > > > > > As it turns out, add_interrupt_randomness() isn't actually used on ARM. > > > > It's certainly called on arm64, per a warning I just hacked in: [...] > > ... and I couldn't immediately spot why 32-bit arm would be different. > > Hmm, I actually meant both arm64 and ARM. > > Marc looked into this at my request a while ago, and I had a look > myself as well at the time, and IIRC, we both concluded that we don't > hit that code path. Darn. > > In any case, the way add_interrupt_randomness() calls > arch_get_random_seed_long() is absolutely insane, so we should try to > fix that in any case. I have no strong opinion there, and I'm happy with that getting cleaned up. Regardless, I do think it's reasonable for the common code to expect that arch_get_random_*() to be roughly as expensive as "most other instructions" (since even RNDR* is expensive the CPU might be able to do useful speculative work in the mean time), whereas a trap to the host is always liable to be expensive as no useful work can be done while the host is handling it, so I think it makes sense to distinguish the two. Thanks, Mark. ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v2 4/5] arm64: Add support for SMCCC TRNG entropy source
On Thu, Nov 05, 2020 at 02:29:49PM +, Mark Brown wrote: > On Thu, Nov 05, 2020 at 02:03:22PM +, Mark Rutland wrote: > > On Thu, Nov 05, 2020 at 01:41:42PM +, Mark Brown wrote: > > > > It isn't obvious to me why we don't fall through to trying the SMCCC > > > TRNG here if for some reason the v8.5-RNG didn't give us something. > > > Definitely an obscure possibility but still... > > > I think it's better to assume that if we have a HW RNG and it's not > > giving us entropy, it's not worthwhile trapping to the host, which might > > encounter the exact same issue. > > There's definitely a good argument for that, but OTOH it's possible the > SMCCC implementation is doing something else (it'd be an interesting > implementation decision but...). That said I don't really mind, I think > my comment was more that if we're doing this the code should be explicit > about what the intent is since right now it isn't obvious. Either a > comment or having an explicit "what method are we choosing" thing. > > > That said, I'm not sure it's great to plumb this under the > > arch_get_random*() interfaces, e.g. given this measn that > > add_interrupt_randomness() will end up trapping to the host all the time > > when it calls arch_get_random_seed_long(). > > > Is there an existing interface for "slow" runtime entropy that we can > > plumb this into instead? > > Yeah, I was wondering about this myself - it seems like a better fit for > hwrng rather than the arch interfaces but that's not used until > userspace comes up, the arch stuff is all expected to be quick. I > suppose we could implement the SMCCC stuff for the early variants of the > API you added so it gets used for bootstrapping purposes and then we > rely on userspace keeping things topped up by fetching entropy through > hwrng or otherwise but that feels confused so I have a hard time getting > enthusiastic about it. I'm perfectly happy for the early functions to call this, or for us to add something new firmwware_get_random_*() functions that we can call early (and potentially at runtime, but less often than arch_get_random_*()). I suspect the easy thing to do for now is plumb this into the existing early arch functions and hwrng. Thanks, Mark. ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v2 4/5] arm64: Add support for SMCCC TRNG entropy source
On Thu, 5 Nov 2020 at 15:30, Mark Rutland wrote: > > On Thu, Nov 05, 2020 at 03:04:57PM +0100, Ard Biesheuvel wrote: > > On Thu, 5 Nov 2020 at 15:03, Mark Rutland wrote: > > > On Thu, Nov 05, 2020 at 01:41:42PM +, Mark Brown wrote: > > > > On Thu, Nov 05, 2020 at 12:56:55PM +, Andre Przywara wrote: > > > > That said, I'm not sure it's great to plumb this under the > > > arch_get_random*() interfaces, e.g. given this measn that > > > add_interrupt_randomness() will end up trapping to the host all the time > > > when it calls arch_get_random_seed_long(). > > > > As it turns out, add_interrupt_randomness() isn't actually used on ARM. > > It's certainly called on arm64, per a warning I just hacked in: > > [1.083802] [ cut here ] > [1.084802] add_interrupt_randomness called > [1.085685] WARNING: CPU: 1 PID: 0 at drivers/char/random.c:1267 > add_interrupt_randomness+0x2e8/0x318 > [1.087599] Modules linked in: > [1.088258] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 5.10.0-rc2-dirty #13 > [1.089672] Hardware name: linux,dummy-virt (DT) > [1.090659] pstate: 60400085 (nZCv daIf +PAN -UAO -TCO BTYPE=--) > [1.091910] pc : add_interrupt_randomness+0x2e8/0x318 > [1.092965] lr : add_interrupt_randomness+0x2e8/0x318 > [1.094021] sp : 80001000be80 > [1.094732] x29: 80001000be80 x28: 2d0c80209840 > [1.095859] x27: 137c3e3a x26: 8000100abdd0 > [1.096978] x25: 0035 x24: 67918bda8000 > [1.098100] x23: c57c31923fe8 x22: fffedc14 > [1.099224] x21: 2d0dbef796a0 x20: c57c331d16a0 > [1.100339] x19: c57c33720a48 x18: 0010 > [1.101459] x17: x16: 0002 > [1.102578] x15: 00e7 x14: 80001000bb20 > [1.103706] x13: ffea x12: c57c337b56e8 > [1.104821] x11: 0003 x10: c57c3379d6a8 > [1.105944] x9 : c57c3379d700 x8 : 00017fe8 > [1.107073] x7 : c000efff x6 : 0001 > [1.108186] x5 : 00057fa8 x4 : > [1.109305] x3 : x2 : c57c337455d0 > [1.110428] x1 : db8dc9c2a1e0f600 x0 : > [1.111552] Call trace: > [1.112083] add_interrupt_randomness+0x2e8/0x318 > [1.113074] handle_irq_event_percpu+0x48/0x90 > [1.114016] handle_irq_event+0x48/0xf8 > [1.114826] handle_fasteoi_irq+0xa4/0x130 > [1.115689] generic_handle_irq+0x30/0x48 > [1.116528] __handle_domain_irq+0x64/0xc0 > [1.117392] gic_handle_irq+0xc0/0x138 > [1.118194] el1_irq+0xbc/0x180 > [1.118870] arch_cpu_idle+0x20/0x30 > [1.119630] default_idle_call+0x8c/0x350 > [1.120479] do_idle+0x224/0x298 > [1.121163] cpu_startup_entry+0x28/0x70 > [1.121994] secondary_start_kernel+0x184/0x198 > > ... and I couldn't immediately spot why 32-bit arm would be different. > Hmm, I actually meant both arm64 and ARM. Marc looked into this at my request a while ago, and I had a look myself as well at the time, and IIRC, we both concluded that we don't hit that code path. Darn. In any case, the way add_interrupt_randomness() calls arch_get_random_seed_long() is absolutely insane, so we should try to fix that in any case. ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v2 4/5] arm64: Add support for SMCCC TRNG entropy source
On 05/11/2020 14:03, Mark Rutland wrote:
> On Thu, Nov 05, 2020 at 01:41:42PM +, Mark Brown wrote:
>> On Thu, Nov 05, 2020 at 12:56:55PM +, Andre Przywara wrote:
>>
>>> static inline bool __must_check arch_get_random_seed_int(unsigned int *v)
>>> {
>>> + struct arm_smccc_res res;
>>> unsigned long val;
>>> - bool ok = arch_get_random_seed_long();
>>>
>>> - *v = val;
>>> - return ok;
>>> + if (cpus_have_const_cap(ARM64_HAS_RNG)) {
>>> + if (arch_get_random_seed_long()) {
>>> + *v = val;
>>> + return true;
>>> + }
>>> + return false;
>>> + }
>>
>> It isn't obvious to me why we don't fall through to trying the SMCCC
>> TRNG here if for some reason the v8.5-RNG didn't give us something.
>> Definitely an obscure possibility but still...
>
> I think it's better to assume that if we have a HW RNG and it's not
> giving us entropy, it's not worthwhile trapping to the host, which might
> encounter the exact same issue.
>
> I'd rather we have one RNG source that we trust works, and use that
> exclusively.
>
> That said, I'm not sure it's great to plumb this under the
> arch_get_random*() interfaces, e.g. given this measn that
> add_interrupt_randomness() will end up trapping to the host all the time
> when it calls arch_get_random_seed_long().
>
> Is there an existing interface for "slow" runtime entropy that we can
> plumb this into instead?
There is the framework implementing /dev/hwrng, and in fact I started
with a driver for that (have that in some working state).
But this is only available somewhat late in the game (after drivers get
initialised), and Ard mentioned that one advantage of the firmware i/f
is (somewhat) early availability. Now for SMCCC we need firmware tables
(for the conduit), so it's not too early either.
If too frequent firmware traps are a concern, we could always request
the maximum 192 bits, and store them. That would avoid 2/3 of the
current traps.
Cheers,
Andre
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v2 4/5] arm64: Add support for SMCCC TRNG entropy source
On Thu, Nov 05, 2020 at 03:04:57PM +0100, Ard Biesheuvel wrote: > On Thu, 5 Nov 2020 at 15:03, Mark Rutland wrote: > > On Thu, Nov 05, 2020 at 01:41:42PM +, Mark Brown wrote: > > > On Thu, Nov 05, 2020 at 12:56:55PM +, Andre Przywara wrote: > > That said, I'm not sure it's great to plumb this under the > > arch_get_random*() interfaces, e.g. given this measn that > > add_interrupt_randomness() will end up trapping to the host all the time > > when it calls arch_get_random_seed_long(). > > As it turns out, add_interrupt_randomness() isn't actually used on ARM. It's certainly called on arm64, per a warning I just hacked in: [1.083802] [ cut here ] [1.084802] add_interrupt_randomness called [1.085685] WARNING: CPU: 1 PID: 0 at drivers/char/random.c:1267 add_interrupt_randomness+0x2e8/0x318 [1.087599] Modules linked in: [1.088258] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 5.10.0-rc2-dirty #13 [1.089672] Hardware name: linux,dummy-virt (DT) [1.090659] pstate: 60400085 (nZCv daIf +PAN -UAO -TCO BTYPE=--) [1.091910] pc : add_interrupt_randomness+0x2e8/0x318 [1.092965] lr : add_interrupt_randomness+0x2e8/0x318 [1.094021] sp : 80001000be80 [1.094732] x29: 80001000be80 x28: 2d0c80209840 [1.095859] x27: 137c3e3a x26: 8000100abdd0 [1.096978] x25: 0035 x24: 67918bda8000 [1.098100] x23: c57c31923fe8 x22: fffedc14 [1.099224] x21: 2d0dbef796a0 x20: c57c331d16a0 [1.100339] x19: c57c33720a48 x18: 0010 [1.101459] x17: x16: 0002 [1.102578] x15: 00e7 x14: 80001000bb20 [1.103706] x13: ffea x12: c57c337b56e8 [1.104821] x11: 0003 x10: c57c3379d6a8 [1.105944] x9 : c57c3379d700 x8 : 00017fe8 [1.107073] x7 : c000efff x6 : 0001 [1.108186] x5 : 00057fa8 x4 : [1.109305] x3 : x2 : c57c337455d0 [1.110428] x1 : db8dc9c2a1e0f600 x0 : [1.111552] Call trace: [1.112083] add_interrupt_randomness+0x2e8/0x318 [1.113074] handle_irq_event_percpu+0x48/0x90 [1.114016] handle_irq_event+0x48/0xf8 [1.114826] handle_fasteoi_irq+0xa4/0x130 [1.115689] generic_handle_irq+0x30/0x48 [1.116528] __handle_domain_irq+0x64/0xc0 [1.117392] gic_handle_irq+0xc0/0x138 [1.118194] el1_irq+0xbc/0x180 [1.118870] arch_cpu_idle+0x20/0x30 [1.119630] default_idle_call+0x8c/0x350 [1.120479] do_idle+0x224/0x298 [1.121163] cpu_startup_entry+0x28/0x70 [1.121994] secondary_start_kernel+0x184/0x198 ... and I couldn't immediately spot why 32-bit arm would be different. Thanks, Mark. ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v2 4/5] arm64: Add support for SMCCC TRNG entropy source
On Thu, Nov 05, 2020 at 02:03:22PM +, Mark Rutland wrote: > On Thu, Nov 05, 2020 at 01:41:42PM +, Mark Brown wrote: > > It isn't obvious to me why we don't fall through to trying the SMCCC > > TRNG here if for some reason the v8.5-RNG didn't give us something. > > Definitely an obscure possibility but still... > I think it's better to assume that if we have a HW RNG and it's not > giving us entropy, it's not worthwhile trapping to the host, which might > encounter the exact same issue. There's definitely a good argument for that, but OTOH it's possible the SMCCC implementation is doing something else (it'd be an interesting implementation decision but...). That said I don't really mind, I think my comment was more that if we're doing this the code should be explicit about what the intent is since right now it isn't obvious. Either a comment or having an explicit "what method are we choosing" thing. > That said, I'm not sure it's great to plumb this under the > arch_get_random*() interfaces, e.g. given this measn that > add_interrupt_randomness() will end up trapping to the host all the time > when it calls arch_get_random_seed_long(). > Is there an existing interface for "slow" runtime entropy that we can > plumb this into instead? Yeah, I was wondering about this myself - it seems like a better fit for hwrng rather than the arch interfaces but that's not used until userspace comes up, the arch stuff is all expected to be quick. I suppose we could implement the SMCCC stuff for the early variants of the API you added so it gets used for bootstrapping purposes and then we rely on userspace keeping things topped up by fetching entropy through hwrng or otherwise but that feels confused so I have a hard time getting enthusiastic about it. signature.asc Description: PGP signature ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v2 4/5] arm64: Add support for SMCCC TRNG entropy source
On Thu, 5 Nov 2020 at 15:03, Mark Rutland wrote:
>
> On Thu, Nov 05, 2020 at 01:41:42PM +, Mark Brown wrote:
> > On Thu, Nov 05, 2020 at 12:56:55PM +, Andre Przywara wrote:
> >
> > > static inline bool __must_check arch_get_random_seed_int(unsigned int *v)
> > > {
> > > + struct arm_smccc_res res;
> > > unsigned long val;
> > > - bool ok = arch_get_random_seed_long();
> > >
> > > - *v = val;
> > > - return ok;
> > > + if (cpus_have_const_cap(ARM64_HAS_RNG)) {
> > > + if (arch_get_random_seed_long()) {
> > > + *v = val;
> > > + return true;
> > > + }
> > > + return false;
> > > + }
> >
> > It isn't obvious to me why we don't fall through to trying the SMCCC
> > TRNG here if for some reason the v8.5-RNG didn't give us something.
> > Definitely an obscure possibility but still...
>
> I think it's better to assume that if we have a HW RNG and it's not
> giving us entropy, it's not worthwhile trapping to the host, which might
> encounter the exact same issue.
>
> I'd rather we have one RNG source that we trust works, and use that
> exclusively.
>
> That said, I'm not sure it's great to plumb this under the
> arch_get_random*() interfaces, e.g. given this measn that
> add_interrupt_randomness() will end up trapping to the host all the time
> when it calls arch_get_random_seed_long().
>
As it turns out, add_interrupt_randomness() isn't actually used on ARM.
> Is there an existing interface for "slow" runtime entropy that we can
> plumb this into instead?
>
> Thanks,
> Mark.
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v2 4/5] arm64: Add support for SMCCC TRNG entropy source
On Thu, Nov 05, 2020 at 01:41:42PM +, Mark Brown wrote:
> On Thu, Nov 05, 2020 at 12:56:55PM +, Andre Przywara wrote:
>
> > static inline bool __must_check arch_get_random_seed_int(unsigned int *v)
> > {
> > + struct arm_smccc_res res;
> > unsigned long val;
> > - bool ok = arch_get_random_seed_long();
> >
> > - *v = val;
> > - return ok;
> > + if (cpus_have_const_cap(ARM64_HAS_RNG)) {
> > + if (arch_get_random_seed_long()) {
> > + *v = val;
> > + return true;
> > + }
> > + return false;
> > + }
>
> It isn't obvious to me why we don't fall through to trying the SMCCC
> TRNG here if for some reason the v8.5-RNG didn't give us something.
> Definitely an obscure possibility but still...
I think it's better to assume that if we have a HW RNG and it's not
giving us entropy, it's not worthwhile trapping to the host, which might
encounter the exact same issue.
I'd rather we have one RNG source that we trust works, and use that
exclusively.
That said, I'm not sure it's great to plumb this under the
arch_get_random*() interfaces, e.g. given this measn that
add_interrupt_randomness() will end up trapping to the host all the time
when it calls arch_get_random_seed_long().
Is there an existing interface for "slow" runtime entropy that we can
plumb this into instead?
Thanks,
Mark.
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v2 4/5] arm64: Add support for SMCCC TRNG entropy source
On Thu, Nov 05, 2020 at 12:56:55PM +, Andre Przywara wrote:
> static inline bool __must_check arch_get_random_seed_int(unsigned int *v)
> {
> + struct arm_smccc_res res;
> unsigned long val;
> - bool ok = arch_get_random_seed_long();
>
> - *v = val;
> - return ok;
> + if (cpus_have_const_cap(ARM64_HAS_RNG)) {
> + if (arch_get_random_seed_long()) {
> + *v = val;
> + return true;
> + }
> + return false;
> + }
It isn't obvious to me why we don't fall through to trying the SMCCC
TRNG here if for some reason the v8.5-RNG didn't give us something.
Definitely an obscure possibility but still...
signature.asc
Description: PGP signature
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH v2 4/5] arm64: Add support for SMCCC TRNG entropy source
The ARM architected TRNG firmware interface, described in ARM spec
DEN0098, defines an ARM SMCCC based interface to a true random number
generator, provided by firmware.
This can be discovered via the SMCCC >=v1.1 interface, and provides
up to 192 bits of entropy per call.
Hook this SMC call into arm64's arch_get_random_*() implementation,
coming to the rescue when the CPU does not implement the ARM v8.5 RNG
system registers.
For the detection, we piggy back on the PSCI/SMCCC discovery (which gives
us the conduit to use (hvc/smc)), then try to call the
ARM_SMCCC_TRNG_VERSION function, which returns -1 if this interface is
not implemented.
Signed-off-by: Andre Przywara
---
arch/arm64/include/asm/archrandom.h | 53 -
1 file changed, 45 insertions(+), 8 deletions(-)
diff --git a/arch/arm64/include/asm/archrandom.h
b/arch/arm64/include/asm/archrandom.h
index abe07c21da8e..03744f688ca1 100644
--- a/arch/arm64/include/asm/archrandom.h
+++ b/arch/arm64/include/asm/archrandom.h
@@ -4,13 +4,24 @@
#ifdef CONFIG_ARCH_RANDOM
+#include
#include
#include
#include
+#define ARM_SMCCC_TRNG_MIN_VERSION 0x1UL
+
+extern bool smccc_trng_available;
+
static inline bool __init smccc_probe_trng(void)
{
- return false;
+ struct arm_smccc_res res;
+
+ arm_smccc_1_1_invoke(ARM_SMCCC_TRNG_VERSION, );
+ if ((s32)res.a0 < 0)
+ return false;
+
+ return res.a0 >= ARM_SMCCC_TRNG_MIN_VERSION;
}
static inline bool __arm64_rndr(unsigned long *v)
@@ -43,26 +54,52 @@ static inline bool __must_check
arch_get_random_int(unsigned int *v)
static inline bool __must_check arch_get_random_seed_long(unsigned long *v)
{
+ struct arm_smccc_res res;
+
/*
* Only support the generic interface after we have detected
* the system wide capability, avoiding complexity with the
* cpufeature code and with potential scheduling between CPUs
* with and without the feature.
*/
- if (!cpus_have_const_cap(ARM64_HAS_RNG))
- return false;
+ if (cpus_have_const_cap(ARM64_HAS_RNG))
+ return __arm64_rndr(v);
- return __arm64_rndr(v);
-}
+ if (smccc_trng_available) {
+ arm_smccc_1_1_invoke(ARM_SMCCC_TRNG_RND64, 64, );
+ if ((int)res.a0 < 0)
+ return false;
+ *v = res.a3;
+ return true;
+ }
+
+ return false;
+}
static inline bool __must_check arch_get_random_seed_int(unsigned int *v)
{
+ struct arm_smccc_res res;
unsigned long val;
- bool ok = arch_get_random_seed_long();
- *v = val;
- return ok;
+ if (cpus_have_const_cap(ARM64_HAS_RNG)) {
+ if (arch_get_random_seed_long()) {
+ *v = val;
+ return true;
+ }
+ return false;
+ }
+
+ if (smccc_trng_available) {
+ arm_smccc_1_1_invoke(ARM_SMCCC_TRNG_RND64, 32, );
+ if ((int)res.a0 < 0)
+ return false;
+
+ *v = res.a3 & GENMASK(31, 0);
+ return true;
+ }
+
+ return false;
}
static inline bool __init __early_cpu_has_rndr(void)
--
2.17.1
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
