qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Philippe Mathieu-Daudé
Subject: Re: [PATCH v2] mips/malta: pass RNG seed to to kernel via env var
Date: Tue, 4 Oct 2022 00:36:03 +0200
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0) Gecko/20100101 Thunderbird/91.13.1

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;




reply via email to

[Prev in Thread] Current Thread [Next in Thread]