Re: [PATCH v2] mips/malta: pass RNG seed to to kernel via env var

Re: [PATCH v2] mips/malta: pass RNG seed to to kernel via env var

[Philippe Mathieu-Daudé]

Hi Jason,

Per 
https://www.qemu.org/docs/master/devel/submitting-a-patch.html#when-resending-patches-add-a-version-tag:

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.

On 3/10/22 12:36, Jason A. Donenfeld wrote:
> As of the kernel commit linked below, Linux ingests an RNG seed
> passed from the hypervisor. So, pass this for the Malta platform, and
> reinitialize it on reboot too, so that it's always fresh.
 >
> Cc: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Cc: Jiaxun Yang <jiaxun.yang@flygoat.com>
> Cc: Aurelien Jarno <aurelien@aurel32.net>
> Link: https://git.kernel.org/mips/c/056a68cea01

You seem to justify this commit by the kernel commit, which justifies
itself mentioning hypervisor use... So the egg comes first before the
chicken.

> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> ---
> Changes v1->v2:
> - Update commit message.
> - No code changes.
> 
>   hw/mips/malta.c | 25 +++++++++++++++++++++++++
>   1 file changed, 25 insertions(+)
> 
> diff --git a/hw/mips/malta.c b/hw/mips/malta.c
> index 0e932988e0..9d793b3c17 100644
> --- a/hw/mips/malta.c
> +++ b/hw/mips/malta.c
> @@ -26,6 +26,7 @@
>   #include "qemu/units.h"
>   #include "qemu/bitops.h"
>   #include "qemu/datadir.h"
> +#include "qemu/guest-random.h"
>   #include "hw/clock.h"
>   #include "hw/southbridge/piix.h"
>   #include "hw/isa/superio.h"
> @@ -1017,6 +1018,17 @@ static void G_GNUC_PRINTF(3, 4) prom_set(uint32_t *prom_buf, int index,
>       va_end(ap);
>   }
>   
> +static void reinitialize_rng_seed(void *opaque)
> +{
> +    char *rng_seed_hex = opaque;
> +    uint8_t rng_seed[32];
> +
> +    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]);
> +    }
> +}
> +
>   /* Kernel */
>   static uint64_t load_kernel(void)
>   {
> @@ -1028,6 +1040,8 @@ static uint64_t load_kernel(void)
>       long prom_size;
>       int prom_index = 0;
>       uint64_t (*xlate_to_kseg0) (void *opaque, uint64_t addr);
> +    uint8_t rng_seed[32];
> +    char rng_seed_hex[sizeof(rng_seed) * 2 + 1];
>   
>   #if TARGET_BIG_ENDIAN
>       big_endian = 1;
> @@ -1115,9 +1129,20 @@ static uint64_t load_kernel(void)
>   
>       prom_set(prom_buf, prom_index++, "modetty0");
>       prom_set(prom_buf, prom_index++, "38400n8r");
> +
> +    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?

Regards,

Phil.

>       prom_set(prom_buf, prom_index++, NULL);
>   
>       rom_add_blob_fixed("prom", prom_buf, prom_size, ENVP_PADDR);
> +    qemu_register_reset(reinitialize_rng_seed,
> +                        memmem(rom_ptr(ENVP_PADDR, prom_size), prom_size,
> +                               rng_seed_hex, sizeof(rng_seed_hex)));
>   
>       g_free(prom_buf);
>       return kernel_entry;

Re: [PATCH v2] mips/malta: pass RNG seed to to kernel via env var

[Jason A. Donenfeld]

Hi Philippe,

On Tue, Oct 4, 2022 at 12:36 AM Philippe Mathieu-Daudé <f4bug@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

Re: [PATCH v2] mips/malta: pass RNG seed to to kernel via env var

[Jason A. Donenfeld]

And just to give you some idea that this truly is possible from firmware
and I'm not just making it up, consider this patch to U-Boot:

u-boot:
diff --git a/arch/mips/lib/bootm.c b/arch/mips/lib/bootm.c
index cab8da4860..27f3ee68c0 100644
--- a/arch/mips/lib/bootm.c
+++ b/arch/mips/lib/bootm.c
@@ -211,6 +211,8 @@ static void linux_env_legacy(bootm_headers_t *images)
 		sprintf(env_buf, "%un8r", gd->baudrate);
 		linux_env_set("modetty0", env_buf);
 	}
+
+	linux_env_set("rngseed", "4142434445464748494a4b4c4d4e4f505152535455565758595a5b5c5d5e5f60");
 }

 static int boot_reloc_fdt(bootm_headers_t *images)

Now, obviously that seed should be generated from some real method (a
seed file in flash, a hardware RNG U-Boot knows about, etc), but for the
purposes of showing that this is how things are passed to Linux, the
above suffices. To show that this ingested by Linux, let's then add:

linux:
diff --git a/drivers/char/random.c b/drivers/char/random.c
index a007e3dad80f..05d5b8bcb7e9 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -890,6 +890,7 @@ EXPORT_SYMBOL_GPL(add_hwgenerator_randomness);
  */
 void __init add_bootloader_randomness(const void *buf, size_t len)
 {
+	print_hex_dump(KERN_ERR, "SARU seed: ", DUMP_PREFIX_OFFSET, 16, 1, buf, len, 1);
 	mix_pool_bytes(buf, len);
 	if (trust_bootloader)
 		credit_init_bits(len * 8);

And now let's boot it:

$ qemu-system-mips -nographic -bios ./u-boot.bin -m 1G -netdev user,tftp=arch/mips/boot,bootfile=/uImage,id=net -device pcnet,netdev=net

U-Boot 2022.10-dirty (Oct 04 2022 - 12:31:05 +0200)

Board: MIPS Malta CoreLV
DRAM:  256 MiB
Core:  3 devices, 3 uclasses, devicetree: separate
PCI: Failed autoconfig bar 10
PCI: Failed autoconfig bar 14
PCI: Failed autoconfig bar 18
PCI: Failed autoconfig bar 1c
PCI: Failed autoconfig bar 20
PCI: Failed autoconfig bar 24
Flash: 4 MiB
Loading Environment from Flash... *** Warning - bad CRC, using default environment

In:    serial@3f8
Out:   serial@3f8
Err:   serial@3f8
Net:   eth0: pcnet#0
IDE:   Bus 0: not available
malta # bootp
BOOTP broadcast 1
DHCP client bound to address 10.0.2.15 (1 ms)
Using pcnet#0 device
TFTP from server 10.0.2.2; our IP address is 10.0.2.15
Filename '/uImage'.
Load address: 0x81000000
Loading: #################################################################
         #################################################################
         #################################################################
         #################################################################
         ####################################################
         169.6 MiB/s
done
Bytes transferred = 4446702 (43d9ee hex)
malta # bootm
## Booting kernel from Legacy Image at 81000000 ...
   Image Name:   Linux-6.0.0-rc6+
   Created:      2022-10-04  10:23:27 UTC
   Image Type:   MIPS Linux Kernel Image (gzip compressed)
   Data Size:    4446638 Bytes = 4.2 MiB
   Load Address: 80100000
   Entry Point:  8054939c
   Verifying Checksum ... OK
   Uncompressing Kernel Image
[    0.000000] Linux version 6.0.0-rc6+ (zx2c4@thinkpad) (mips-linux-musl-gcc (GCC) 11.2.1 20211120, GNU ld (GNU Binutils) 2.37) #5 SMP PREEMPT Fri Jun 5 15:58:00 CEST 2015
[    0.000000] earlycon: uart8250 at I/O port 0x3f8 (options '38400n8')
[    0.000000] printk: bootconsole [uart8250] enabled
[    0.000000] Config serial console: console=ttyS0,38400n8r
[    0.000000] MIPS CPS SMP unable to proceed without a CM
[    0.000000] CPU0 revision is: 00019300 (MIPS 24Kc)
[    0.000000] FPU revision is: 00739300
[    0.000000] OF: fdt: No chosen node found, continuing without
[    0.000000] OF: fdt: Ignoring memory range 0x100000000 - 0x17ffff000
[    0.000000] MIPS: machine is mti,malta
[    0.000000] Software DMA cache coherency enabled
[    0.000000] Initrd not found or empty - disabling initrd
[    0.000000] Primary instruction cache 2kB, VIPT, 2-way, linesize 16 bytes.
[    0.000000] Primary data cache 2kB, 2-way, VIPT, no aliases, linesize 16 bytes
[    0.000000] Zone ranges:
[    0.000000]   DMA      [mem 0x0000000000000000-0x0000000000ffffff]
[    0.000000]   Normal   [mem 0x0000000001000000-0x000000001fffffff]
[    0.000000] Movable zone start for each node
[    0.000000] Early memory node ranges
[    0.000000]   node   0: [mem 0x0000000000000000-0x000000000fffffff]
[    0.000000]   node   0: [mem 0x0000000090000000-0x00000000ffffefff]
[    0.000000] Initmem setup node 0 [mem 0x0000000000000000-0x00000000ffffefff]
[    0.000000] SARU seed: 00000000: 41 42 43 44 45 46 47 48 49 4a 4b 4c 4d 4e 4f 50  ABCDEFGHIJKLMNOP
[    0.000000] SARU seed: 00000010: 51 52 53 54 55 56 57 58 59 5a 5b 5c 5d 5e 5f 60  QRSTUVWXYZ[\]^_`
[    0.000000] random: crng init done
...

So, as you can see, it works perfectly. Thus, setting this in QEMU
follows *exactly* *the* *same* *pattern* as every other architecture
that allows for this kind of mechanism. There's nothing weird or unusual
or out of place happening here.

Hope this helps clarify.

Regards,
Jason

Re: [PATCH v2] mips/malta: pass RNG seed to to kernel via env var

[Peter Maydell]

On Tue, 4 Oct 2022 at 11:40, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> And just to give you some idea that this truly is possible from firmware
> and I'm not just making it up, consider this patch to U-Boot:
>
> u-boot:
> diff --git a/arch/mips/lib/bootm.c b/arch/mips/lib/bootm.c
> index cab8da4860..27f3ee68c0 100644
> --- a/arch/mips/lib/bootm.c
> +++ b/arch/mips/lib/bootm.c
> @@ -211,6 +211,8 @@ static void linux_env_legacy(bootm_headers_t *images)
>                 sprintf(env_buf, "%un8r", gd->baudrate);
>                 linux_env_set("modetty0", env_buf);
>         }
> +
> +       linux_env_set("rngseed", "4142434445464748494a4b4c4d4e4f505152535455565758595a5b5c5d5e5f60");
>  }
>

>
> So, as you can see, it works perfectly. Thus, setting this in QEMU
> follows *exactly* *the* *same* *pattern* as every other architecture
> that allows for this kind of mechanism. There's nothing weird or unusual
> or out of place happening here.

I think the unusual thing here is that this patch isn't
"this facility is implemented by u-boot [commit whatever,
docs whatever], and here is the patch adding it to QEMU's
handling of the same interface". That is, for boards like
Malta the general expectation is that we're emulating
a piece of real hardware and the firmware/bootloader
that it would be running, so "this is a patch that
implements an interface that the real bootloader doesn't
have" is a bit odd.

thanks
-- PMM

Re: [PATCH v2] mips/malta: pass RNG seed to to kernel via env var

[Jason A. Donenfeld]

On Tue, Oct 4, 2022 at 12:53 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Tue, 4 Oct 2022 at 11:40, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> >
> > And just to give you some idea that this truly is possible from firmware
> > and I'm not just making it up, consider this patch to U-Boot:
> >
> > u-boot:
> > diff --git a/arch/mips/lib/bootm.c b/arch/mips/lib/bootm.c
> > index cab8da4860..27f3ee68c0 100644
> > --- a/arch/mips/lib/bootm.c
> > +++ b/arch/mips/lib/bootm.c
> > @@ -211,6 +211,8 @@ static void linux_env_legacy(bootm_headers_t *images)
> >                 sprintf(env_buf, "%un8r", gd->baudrate);
> >                 linux_env_set("modetty0", env_buf);
> >         }
> > +
> > +       linux_env_set("rngseed", "4142434445464748494a4b4c4d4e4f505152535455565758595a5b5c5d5e5f60");
> >  }
> >
>
> >
> > So, as you can see, it works perfectly. Thus, setting this in QEMU
> > follows *exactly* *the* *same* *pattern* as every other architecture
> > that allows for this kind of mechanism. There's nothing weird or unusual
> > or out of place happening here.
>
> I think the unusual thing here is that this patch isn't
> "this facility is implemented by u-boot [commit whatever,
> docs whatever], and here is the patch adding it to QEMU's
> handling of the same interface". That is, for boards like
> Malta the general expectation is that we're emulating
> a piece of real hardware and the firmware/bootloader
> that it would be running, so "this is a patch that
> implements an interface that the real bootloader doesn't
> have" is a bit odd.

Except it's not different from other platforms that get bootloader
seeds as such. A bootloader can easily pass this; QEMU most certainly
should pass this. (I sincerely hope you're not arguing in favor of
holding back progress in this area for yet another decade.)

Jason

Re: [PATCH v2] mips/malta: pass RNG seed to to kernel via env var

[Jason A. Donenfeld]

On Tue, Oct 4, 2022 at 12:56 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> > > So, as you can see, it works perfectly. Thus, setting this in QEMU
> > > follows *exactly* *the* *same* *pattern* as every other architecture
> > > that allows for this kind of mechanism. There's nothing weird or unusual
> > > or out of place happening here.
> >
> > I think the unusual thing here is that this patch isn't
> > "this facility is implemented by u-boot [commit whatever,
> > docs whatever], and here is the patch adding it to QEMU's
> > handling of the same interface". That is, for boards like
> > Malta the general expectation is that we're emulating
> > a piece of real hardware and the firmware/bootloader
> > that it would be running, so "this is a patch that
> > implements an interface that the real bootloader doesn't
> > have" is a bit odd.
>
> Except it's not different from other platforms that get bootloader
> seeds as such. A bootloader can easily pass this; QEMU most certainly
> should pass this. (I sincerely hope you're not arguing in favor of
> holding back progress in this area for yet another decade.)

Also, in case you've missed the actual context of this patch, it
happens for `-kernel ...`. So we're now strictly in the realm of QEMU
things. This is how things work on all platforms. If you use
`-kernel`, then QEMU will put things in the right place to directly
boot the kernel, or pass some prepared blob to an option ROM, or
whatever else. This isn't related to running `-bios u-boot.bin`,
except insofar as U-Boot can implement the same thing, as I've shown.

Jason

Re: [PATCH v2] mips/malta: pass RNG seed to to kernel via env var

[Peter Maydell]

On Tue, 4 Oct 2022 at 11:56, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> On Tue, Oct 4, 2022 at 12:53 PM Peter Maydell <peter.maydell@linaro.org> wrote:
> >
> > On Tue, 4 Oct 2022 at 11:40, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> > I think the unusual thing here is that this patch isn't
> > "this facility is implemented by u-boot [commit whatever,
> > docs whatever], and here is the patch adding it to QEMU's
> > handling of the same interface". That is, for boards like
> > Malta the general expectation is that we're emulating
> > a piece of real hardware and the firmware/bootloader
> > that it would be running, so "this is a patch that
> > implements an interface that the real bootloader doesn't
> > have" is a bit odd.
>
> Except it's not different from other platforms that get bootloader
> seeds as such. A bootloader can easily pass this; QEMU most certainly
> should pass this. (I sincerely hope you're not arguing in favor of
> holding back progress in this area for yet another decade.)

What I'm asking, I guess, is why you're messing with this board
model at all if you haven't added this functionality to u-boot.
This is just an emulation of an ancient bit of MIPS hardware, which
nobody really cares about very much I hope.

I'm not saying this is a bad patch -- I'm just saying that
QEMU should not be in the business of defining bootloader-to-kernel
interfaces if it can avoid it, so usually the expectation is
that we are just implementing interfaces that are already
defined, documented and implemented by a real bootloader and kernel.

[from your other mail]
> Also, in case you've missed the actual context of this patch, it
> happens for `-kernel ...`. So we're now strictly in the realm of QEMU
> things.

-kernel generally means "emulate the platform's boot loader":
it is exactly because we do not want to be in a realm of
QEMU-defined interfaces that we try to follow what the
platform boot loader does rather than defining new
interfaces. That's not always possible or the right thing,
but it's usually the cleaner way to go.

thanks
-- PMM

Re: [PATCH v2] mips/malta: pass RNG seed to to kernel via env var

[Jason A. Donenfeld]

On Tue, Oct 4, 2022 at 1:03 PM Peter Maydell <peter.maydell@linaro.org> wrote:
> What I'm asking, I guess, is why you're messing with this board
> model at all if you haven't added this functionality to u-boot.
> This is just an emulation of an ancient bit of MIPS hardware, which
> nobody really cares about very much I hope.

I think most people emulating MIPS would disagree. This is basically a
reference platform for most intents and purposes. As I mentioned, this
involves `-kernel` -- the thing that's used when you explicitly opt-in
to not using a bootloader, so when you sign up for QEMU arranging the
kernel image and its environment. Neglecting to pass an RNG seed would
be a grave mistake.

> I'm not saying this is a bad patch -- I'm just saying that
> QEMU should not be in the business of defining bootloader-to-kernel
> interfaces if it can avoid it, so usually the expectation is
> that we are just implementing interfaces that are already
> defined, documented and implemented by a real bootloader and kernel.

Except that's not really the way things have ever worked here. The
kernel now has the "rngseed" env var functionality, which is useful
for a variety of scenarios -- kexec, firmware, and *most importantly*
for QEMU. Don't block progress here.

> -kernel generally means "emulate the platform's boot loader"

And here, a platform bootloader could pass this, just as is the case
with m68k's BI_RNG_SEED or x86's setup_data RNG SEED or device tree's
rng-seed or EFI's LINUX_EFI_RANDOM_SEED_TABLE_GUID or MIPS' "rngseed"
fw environment variable. These are important facilities to have.
Bootloaders and hypervisors alike must implement them. *Do not block
progress here.*

Jason

Re: [PATCH v2] mips/malta: pass RNG seed to to kernel via env var

[BALATON Zoltan]

On Tue, 4 Oct 2022, Jason A. Donenfeld wrote:
> On Tue, Oct 4, 2022 at 1:03 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>> What I'm asking, I guess, is why you're messing with this board
>> model at all if you haven't added this functionality to u-boot.
>> This is just an emulation of an ancient bit of MIPS hardware, which
>> nobody really cares about very much I hope.
>
> I think most people emulating MIPS would disagree. This is basically a
> reference platform for most intents and purposes. As I mentioned, this
> involves `-kernel` -- the thing that's used when you explicitly opt-in
> to not using a bootloader, so when you sign up for QEMU arranging the
> kernel image and its environment. Neglecting to pass an RNG seed would
> be a grave mistake.
>
>> I'm not saying this is a bad patch -- I'm just saying that
>> QEMU should not be in the business of defining bootloader-to-kernel
>> interfaces if it can avoid it, so usually the expectation is
>> that we are just implementing interfaces that are already
>> defined, documented and implemented by a real bootloader and kernel.
>
> Except that's not really the way things have ever worked here. The
> kernel now has the "rngseed" env var functionality, which is useful
> for a variety of scenarios -- kexec, firmware, and *most importantly*
> for QEMU. Don't block progress here.
>
>> -kernel generally means "emulate the platform's boot loader"
>
> And here, a platform bootloader could pass this, just as is the case
> with m68k's BI_RNG_SEED or x86's setup_data RNG SEED or device tree's
> rng-seed or EFI's LINUX_EFI_RANDOM_SEED_TABLE_GUID or MIPS' "rngseed"
> fw environment variable. These are important facilities to have.
> Bootloaders and hypervisors alike must implement them. *Do not block
> progress here.*

Cool dowm. Peter does not want to block progress here. What he said was 
that the malta is (or should be) emulating a real piece of hardware so 
adding some stuff to it which is not on that real hardware may not be 
preferred. If you want to experiment with generic mips hardware maybe you 
need a virt board instead that is free from such restrictions to emulate a 
real hardware. Some archs already have such board and there seems to be 
loongson3-virt but no generic mips virt machine yet. Defining and 
implementing such board may be more than you want to do for this but maybe 
that would be a better way to go.

Regards,
BALATON Zoltan

Re: [PATCH v2] mips/malta: pass RNG seed to to kernel via env var

[Jason A. Donenfeld]

On Tue, Oct 4, 2022 at 1:39 PM BALATON Zoltan <balaton@eik.bme.hu> wrote:
>
> On Tue, 4 Oct 2022, Jason A. Donenfeld wrote:
> > On Tue, Oct 4, 2022 at 1:03 PM Peter Maydell <peter.maydell@linaro.org> wrote:
> >> What I'm asking, I guess, is why you're messing with this board
> >> model at all if you haven't added this functionality to u-boot.
> >> This is just an emulation of an ancient bit of MIPS hardware, which
> >> nobody really cares about very much I hope.
> >
> > I think most people emulating MIPS would disagree. This is basically a
> > reference platform for most intents and purposes. As I mentioned, this
> > involves `-kernel` -- the thing that's used when you explicitly opt-in
> > to not using a bootloader, so when you sign up for QEMU arranging the
> > kernel image and its environment. Neglecting to pass an RNG seed would
> > be a grave mistake.
> >
> >> I'm not saying this is a bad patch -- I'm just saying that
> >> QEMU should not be in the business of defining bootloader-to-kernel
> >> interfaces if it can avoid it, so usually the expectation is
> >> that we are just implementing interfaces that are already
> >> defined, documented and implemented by a real bootloader and kernel.
> >
> > Except that's not really the way things have ever worked here. The
> > kernel now has the "rngseed" env var functionality, which is useful
> > for a variety of scenarios -- kexec, firmware, and *most importantly*
> > for QEMU. Don't block progress here.
> >
> >> -kernel generally means "emulate the platform's boot loader"
> >
> > And here, a platform bootloader could pass this, just as is the case
> > with m68k's BI_RNG_SEED or x86's setup_data RNG SEED or device tree's
> > rng-seed or EFI's LINUX_EFI_RANDOM_SEED_TABLE_GUID or MIPS' "rngseed"
> > fw environment variable. These are important facilities to have.
> > Bootloaders and hypervisors alike must implement them. *Do not block
> > progress here.*
>
> Cool dowm. Peter does not want to block progress here. What he said was
> that the malta is (or should be) emulating a real piece of hardware so
> adding some stuff to it which is not on that real hardware may not be
> preferred. If you want to experiment with generic mips hardware maybe you
> need a virt board instead that is free from such restrictions to emulate a
> real hardware. Some archs already have such board and there seems to be
> loongson3-virt but no generic mips virt machine yet. Defining and
> implementing such board may be more than you want to do for this but maybe
> that would be a better way to go.

This is the bikeshed suggestion that puts along the path of nothing
ever getting done. This is an interface that's available for real
firmware; there's no reason not to implement it here. It's the same
situation as the MIPS boston board setting the rng-seed device tree
property. There's nothing new or unusual about this, and it fits with
how things work elsewhere on the architecture and QEMU at large.
Besides, "malta" is the de facto platform used for emulating MIPS.

Again, this is obvious progress blocking in action. Look how it's done
elsewhere; look at how it's done in this patch; there's no difference.
This patch is boring and unoffensive. We don't need to waste time
bikeshedding it.

Jason

Re: [PATCH v2] mips/malta: pass RNG seed to to kernel via env var

[Daniel P. Berrangé]

On Tue, Oct 04, 2022 at 12:36:03AM +0200, Philippe Mathieu-Daudé wrote:
> Hi Jason,
> 
> Per https://www.qemu.org/docs/master/devel/submitting-a-patch.html#when-resending-patches-add-a-version-tag:
> 
> 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.
> 
> On 3/10/22 12:36, Jason A. Donenfeld wrote:
> > As of the kernel commit linked below, Linux ingests an RNG seed
> > passed from the hypervisor. So, pass this for the Malta platform, and
> > reinitialize it on reboot too, so that it's always fresh.
> >
> > Cc: Philippe Mathieu-Daudé <f4bug@amsat.org>
> > Cc: Jiaxun Yang <jiaxun.yang@flygoat.com>
> > Cc: Aurelien Jarno <aurelien@aurel32.net>
> > Link: https://git.kernel.org/mips/c/056a68cea01
> 
> You seem to justify this commit by the kernel commit, which justifies
> itself mentioning hypervisor use... So the egg comes first before the
> chicken.

The kernel justification is that the guest OS needs a good RNG
seed. The kernel patch is just saying that the firmware / hypervisor
side is where this seed generally expected to come from. This is
fine, and not notably different from what Jason's already got
wired up for the various other targets in QEMU.


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|