From: Ard Biesheuvel <ardb@kernel.org> To: "Jason A. Donenfeld" <Jason@zx2c4.com> Cc: Eric Biggers <ebiggers@kernel.org>, "Theodore Y. Ts'o" <tytso@mit.edu>, Linux Kernel Mailing List <linux-kernel@vger.kernel.org>, Linux ARM <linux-arm-kernel@lists.infradead.org>, Marc Zyngier <maz@kernel.org>, Mark Rutland <mark.rutland@arm.com>, Mark Brown <broonie@kernel.org>, Andre Przywara <andre.przywara@arm.com> Subject: Re: [PATCH] random: avoid arch_get_random_seed_long() when collecting IRQ randomness Date: Mon, 7 Dec 2020 16:35:01 +0100 [thread overview] Message-ID: <CAMj1kXEXa1Gh-RzRFMe-eyXRT6Qdta+PWifm8AWt7YvM4sS=Zg@mail.gmail.com> (raw) In-Reply-To: <CAHmME9pzcxQ1aufU7ycTcL+NQYV8P_wMKpetAuSogOw=2N9jRw@mail.gmail.com> On Mon, 7 Dec 2020 at 15:28, Jason A. Donenfeld <Jason@zx2c4.com> wrote: > > Hi Ard, > > On Tue, Dec 1, 2020 at 1:24 PM Ard Biesheuvel <ardb@kernel.org> wrote: > > > > > is implemented. In most cases, these are special instructions, but in > > > > > some cases, such as on ARM, we may want to back this using firmware > > > > > calls, which are considerably more expensive. > > This seems fine. But I suppose I'm curious to learn more about what > you have in mind for ARM. We've been assuming that arch_get_random is > not horribly expensive. Usually external RNGs that are horribly > expensive separate hardware take a different route and aren't hooked > into arch_get_random. When you say "we may want to back this using > firmware", does that mean it hasn't happened yet, and you're in a > position to direct the design otherwise? If so, would it be reasonable > to take a different route with that hardware, and keep arch_get_random > for when it's actually implemented by the hardware? Or are there > actually good reasons for keeping it one and the same? > Many older generation ARM SoCs have IP blocks that expose an entropy source of some kind, and map it in the normal world, which is accessible by the OS directly. These are driven as hwrngs via the driver stack, which models them as actual devices, with clock and regulator handling, power management hooks, etc etc. There are multiple examples where such a SoC is being revved up with newer cores etc, and now, the IP block is in the secure world, which means the OS cannot access it directly, and needs to issue an SMC instruction to perform a firmware call to access it. The secure world firmware is now entirely in charge of the hardware, and so this SMC call is really the only thing that goes on in this driver (no clocks, regulators, etc) So to prevent fragmentation, as well as make the entropy source available much earlier in the boot, ARM has issued a firmware spec that unifies these SMC calls, and defines them as non-blocking, i.e., return the requested number of entropy bits, or fail immediately. Therefore, this should not be super expensive, given that the only overhead is the CPU cycles spent on calling into the firmware (and a bit of overhead perhaps from poking some MMIO registers in the IP block). But it is definitely not suitable for being called hundreds of times per second, hence this patch. (Note that we are talking about arch_get_random_seed_long() here, not arch_get_random_long()) > On the other hand, rdrand on intel is getting slower and slower, to > the point where we've removed it from a few places that used to use > it. And I don't see anything terribly wrong with removing the extra > call in this path here. So need be, I'll offer my Reviewed-by. But I > wanted to get an understanding of the fuller story first. > Given that we already have both arch_get_random_seed_long() and arch_get_random_long(), I think it is reasonable for the former to be allowed to be slightly more expensive, and we should only invoke it for the purpose of reseeding a pseudo-RNG. If this occurs on a hot path, there is something terribly wrong already, so I don't think RDRAND/RDSEED performance should be a huge concern here. Note that once this patch lands, we also intend to change the current way the arm64 RNDR and RNDRRS instructions are mapped to arch_get_random_seed_long() and arch_get_random_long(). (RNDR returns 64-bit from a DRBG that reseeds an an implementation defined rate, and RNDRRS does the same but forces a reseed to occur first) Thanks, Ard.
WARNING: multiple messages have this Message-ID (diff)
From: Ard Biesheuvel <ardb@kernel.org> To: "Jason A. Donenfeld" <Jason@zx2c4.com> Cc: Mark Rutland <mark.rutland@arm.com>, "Theodore Y. Ts'o" <tytso@mit.edu>, Marc Zyngier <maz@kernel.org>, Linux Kernel Mailing List <linux-kernel@vger.kernel.org>, Eric Biggers <ebiggers@kernel.org>, Mark Brown <broonie@kernel.org>, Andre Przywara <andre.przywara@arm.com>, Linux ARM <linux-arm-kernel@lists.infradead.org> Subject: Re: [PATCH] random: avoid arch_get_random_seed_long() when collecting IRQ randomness Date: Mon, 7 Dec 2020 16:35:01 +0100 [thread overview] Message-ID: <CAMj1kXEXa1Gh-RzRFMe-eyXRT6Qdta+PWifm8AWt7YvM4sS=Zg@mail.gmail.com> (raw) In-Reply-To: <CAHmME9pzcxQ1aufU7ycTcL+NQYV8P_wMKpetAuSogOw=2N9jRw@mail.gmail.com> On Mon, 7 Dec 2020 at 15:28, Jason A. Donenfeld <Jason@zx2c4.com> wrote: > > Hi Ard, > > On Tue, Dec 1, 2020 at 1:24 PM Ard Biesheuvel <ardb@kernel.org> wrote: > > > > > is implemented. In most cases, these are special instructions, but in > > > > > some cases, such as on ARM, we may want to back this using firmware > > > > > calls, which are considerably more expensive. > > This seems fine. But I suppose I'm curious to learn more about what > you have in mind for ARM. We've been assuming that arch_get_random is > not horribly expensive. Usually external RNGs that are horribly > expensive separate hardware take a different route and aren't hooked > into arch_get_random. When you say "we may want to back this using > firmware", does that mean it hasn't happened yet, and you're in a > position to direct the design otherwise? If so, would it be reasonable > to take a different route with that hardware, and keep arch_get_random > for when it's actually implemented by the hardware? Or are there > actually good reasons for keeping it one and the same? > Many older generation ARM SoCs have IP blocks that expose an entropy source of some kind, and map it in the normal world, which is accessible by the OS directly. These are driven as hwrngs via the driver stack, which models them as actual devices, with clock and regulator handling, power management hooks, etc etc. There are multiple examples where such a SoC is being revved up with newer cores etc, and now, the IP block is in the secure world, which means the OS cannot access it directly, and needs to issue an SMC instruction to perform a firmware call to access it. The secure world firmware is now entirely in charge of the hardware, and so this SMC call is really the only thing that goes on in this driver (no clocks, regulators, etc) So to prevent fragmentation, as well as make the entropy source available much earlier in the boot, ARM has issued a firmware spec that unifies these SMC calls, and defines them as non-blocking, i.e., return the requested number of entropy bits, or fail immediately. Therefore, this should not be super expensive, given that the only overhead is the CPU cycles spent on calling into the firmware (and a bit of overhead perhaps from poking some MMIO registers in the IP block). But it is definitely not suitable for being called hundreds of times per second, hence this patch. (Note that we are talking about arch_get_random_seed_long() here, not arch_get_random_long()) > On the other hand, rdrand on intel is getting slower and slower, to > the point where we've removed it from a few places that used to use > it. And I don't see anything terribly wrong with removing the extra > call in this path here. So need be, I'll offer my Reviewed-by. But I > wanted to get an understanding of the fuller story first. > Given that we already have both arch_get_random_seed_long() and arch_get_random_long(), I think it is reasonable for the former to be allowed to be slightly more expensive, and we should only invoke it for the purpose of reseeding a pseudo-RNG. If this occurs on a hot path, there is something terribly wrong already, so I don't think RDRAND/RDSEED performance should be a huge concern here. Note that once this patch lands, we also intend to change the current way the arm64 RNDR and RNDRRS instructions are mapped to arch_get_random_seed_long() and arch_get_random_long(). (RNDR returns 64-bit from a DRBG that reseeds an an implementation defined rate, and RNDRRS does the same but forces a reseed to occur first) Thanks, Ard. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2020-12-07 15:35 UTC|newest] Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-11-05 15:29 [PATCH] random: avoid arch_get_random_seed_long() when collecting IRQ randomness Ard Biesheuvel 2020-11-05 15:29 ` Ard Biesheuvel 2020-11-11 8:19 ` Ard Biesheuvel 2020-11-11 8:19 ` Ard Biesheuvel 2020-11-11 9:45 ` André Przywara 2020-11-11 9:45 ` André Przywara 2020-11-11 10:05 ` Ard Biesheuvel 2020-11-11 10:05 ` Ard Biesheuvel 2020-11-11 10:46 ` André Przywara 2020-11-11 10:46 ` André Przywara 2020-11-11 11:48 ` Ard Biesheuvel 2020-11-11 11:48 ` Ard Biesheuvel 2020-11-17 13:33 ` Ard Biesheuvel 2020-11-17 13:33 ` Ard Biesheuvel 2021-01-04 19:09 ` Ard Biesheuvel 2021-01-04 19:09 ` Ard Biesheuvel 2021-01-10 9:45 ` Ard Biesheuvel 2021-01-10 9:45 ` Ard Biesheuvel 2021-01-10 13:55 ` Jason A. Donenfeld 2021-01-15 13:18 ` Jason A. Donenfeld 2021-01-15 13:18 ` Jason A. Donenfeld 2020-11-20 4:11 ` Eric Biggers 2020-11-20 4:11 ` Eric Biggers 2020-12-01 12:23 ` Ard Biesheuvel 2020-12-01 12:23 ` Ard Biesheuvel 2020-12-07 8:12 ` Ard Biesheuvel 2020-12-07 8:12 ` Ard Biesheuvel 2020-12-07 14:27 ` Jason A. Donenfeld 2020-12-07 14:27 ` Jason A. Donenfeld 2020-12-07 15:35 ` Ard Biesheuvel [this message] 2020-12-07 15:35 ` Ard Biesheuvel 2020-11-20 15:27 ` Marc Zyngier 2020-11-20 15:27 ` Marc Zyngier 2020-11-27 12:08 ` Ard Biesheuvel 2020-11-27 12:08 ` Ard Biesheuvel 2021-01-21 17:53 ` Will Deacon 2021-01-21 17:53 ` Will Deacon
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='CAMj1kXEXa1Gh-RzRFMe-eyXRT6Qdta+PWifm8AWt7YvM4sS=Zg@mail.gmail.com' \ --to=ardb@kernel.org \ --cc=Jason@zx2c4.com \ --cc=andre.przywara@arm.com \ --cc=broonie@kernel.org \ --cc=ebiggers@kernel.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=mark.rutland@arm.com \ --cc=maz@kernel.org \ --cc=tytso@mit.edu \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.