Hi Philippe,
On Tue, Oct 4, 2022 at 12:36 AM Philippe Mathieu-Daudé <f4...@amsat.org> wrote:
> Send each new revision as a new top-level thread, rather than burying it
> in-reply-to an earlier revision, as many reviewers are not looking
> inside deep threads for new patches.
Will do.
> You seem to justify this commit by the kernel commit, which justifies
> itself mentioning hypervisor use... So the egg comes first before the
> chicken.
Oh, that's not really the intention. My goal is to provide sane
interfaces for preboot environments -- whether those are in a
hypervisor like QEMU or in firmware like CFE -- to pass a random seed
along to the kernel. To that end, I've been making sure there's both a
kernel side and a QEMU side, and submitting both to see what folks
think. The fact that you have some questions (below) is a good thing;
I'm glad to have your input on it.
> > +
> > + qemu_guest_getrandom_nofail(rng_seed, sizeof(rng_seed));
> > + for (size_t i = 0; i < sizeof(rng_seed); ++i) {
> > + sprintf(rng_seed_hex + i * 2, "%02x", rng_seed[i]);
> > + }
> > + prom_set(prom_buf, prom_index++, "rngseed");
> > + prom_set(prom_buf, prom_index++, "%s", rng_seed_hex);
>
> You use the firmware interface to pass rng data to an hypervisor...
>
> Look to me you are forcing one API to ease another one. From the
> FW PoV it is a lie, because the FW will only change this value if
> an operator is involved. Here PROM stands for "programmable read-only
> memory", rarely modified. Having the 'rngseed' updated on each
> reset is surprising.
>
> Do you have an example of firmware doing that? (So I can understand
> whether this is the best way to mimic this behavior here).
>
> Aren't they better APIs to have hypervisors pass data to a kernel?
So a firmware interface *is* the intended situation here. To answer
your last question first: the "standard" firmware interface for
passing these seeds is via device tree's "rng-seed" field. There's
also a EFI protocol for this. And on x86 it can be passed through the
setup_data field. And on m68k the bootinfo bootloader/firmware struct
has a BI_RNG_SEED type. There's plenty of ARM and x86 hardware that
uses device tree and EFI for this, where the firmware is involved in
generating the seeds, and in the device tree case, in mangling the
device tree to have the right values. So, to answer your first
question, yes I think this is indeed a firmware-style interface.
Right now this is obviously intended for QEMU (and other hypervisors)
to implement. Later I'm hoping that firmware environments like CFE
might gain support for setting this. (You could do so interactively
now with "setenv".) So it seems like the environment block here really
is the right way to pass this. If you have a MIPS/malta platform
alternative, I'd be happy to consider it with you, but in my look at
things so far, the fw env block seems like by far the best way of
doing this, especially so considering it's part of both real firmware
environments and QEMU, and is relatively straightforward to implement.
Jason
