[PATCH v3 0/8] rerandomize RNG seeds on reboot and handle record&replay

[PATCH v3 0/8] rerandomize RNG seeds on reboot and handle record&replay

[Jason A. Donenfeld]

When the system reboots, the rng seed that QEMU passes should be
re-randomized, so that the new boot gets a new seed. This series wires
that up for FDT.

Then, since the record&replay subsystem makes use of reset as well, we
add a new reset cause for record&replay, so that we can avoid
re-randomizing in these cases.

Jason A. Donenfeld (8):
  reset: allow registering handlers that aren't called by snapshot
    loading
  x86: do not re-randomize RNG seed on snapshot load
  device-tree: add re-randomization helper function
  arm: re-randomize rng-seed on reboot
  riscv: re-randomize rng-seed on reboot
  openrisc: re-randomize rng-seed on reboot
  rx: re-randomize rng-seed on reboot
  mips: re-randomize rng-seed on reboot

 hw/arm/aspeed.c              |  4 ++--
 hw/arm/boot.c                |  2 ++
 hw/arm/mps2-tz.c             |  4 ++--
 hw/core/reset.c              | 15 ++++++++++++++-
 hw/hppa/machine.c            |  4 ++--
 hw/i386/microvm.c            |  4 ++--
 hw/i386/pc.c                 |  6 +++---
 hw/i386/x86.c                |  2 +-
 hw/mips/boston.c             |  3 +++
 hw/openrisc/boot.c           |  3 +++
 hw/ppc/pegasos2.c            |  4 ++--
 hw/ppc/pnv.c                 |  4 ++--
 hw/ppc/spapr.c               |  4 ++--
 hw/riscv/boot.c              |  3 +++
 hw/rx/rx-gdbsim.c            |  3 +++
 hw/s390x/s390-virtio-ccw.c   |  4 ++--
 include/hw/boards.h          |  2 +-
 include/sysemu/device_tree.h |  9 +++++++++
 include/sysemu/reset.h       |  5 ++++-
 migration/savevm.c           |  2 +-
 qapi/run-state.json          |  4 +++-
 softmmu/device_tree.c        | 21 +++++++++++++++++++++
 softmmu/runstate.c           |  4 ++--
 23 files changed, 89 insertions(+), 27 deletions(-)

-- 
2.37.3

[PATCH v3 1/8] reset: allow registering handlers that aren't called by snapshot loading

[Jason A. Donenfeld]

Snapshot loading only expects to call deterministic handlers, not
non-deterministic ones. So introduce a way of registering handlers that
won't be called when reseting for snapshots.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 hw/arm/aspeed.c            |  4 ++--
 hw/arm/mps2-tz.c           |  4 ++--
 hw/core/reset.c            | 15 ++++++++++++++-
 hw/hppa/machine.c          |  4 ++--
 hw/i386/microvm.c          |  4 ++--
 hw/i386/pc.c               |  6 +++---
 hw/ppc/pegasos2.c          |  4 ++--
 hw/ppc/pnv.c               |  4 ++--
 hw/ppc/spapr.c             |  4 ++--
 hw/s390x/s390-virtio-ccw.c |  4 ++--
 include/hw/boards.h        |  2 +-
 include/sysemu/reset.h     |  5 ++++-
 migration/savevm.c         |  2 +-
 qapi/run-state.json        |  4 +++-
 softmmu/runstate.c         |  4 ++--
 15 files changed, 44 insertions(+), 26 deletions(-)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index bc3ecdb619..69cadb1c37 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -1349,12 +1349,12 @@ static void aspeed_machine_bletchley_class_init(ObjectClass *oc, void *data)
         aspeed_soc_num_cpus(amc->soc_name);
 }
 
-static void fby35_reset(MachineState *state)
+static void fby35_reset(MachineState *state, ShutdownCause reason)
 {
     AspeedMachineState *bmc = ASPEED_MACHINE(state);
     AspeedGPIOState *gpio = &bmc->soc.gpio;
 
-    qemu_devices_reset();
+    qemu_devices_reset(reason);
 
     /* Board ID: 7 (Class-1, 4 slots) */
     object_property_set_bool(OBJECT(gpio), "gpioV4", true, &error_fatal);
diff --git a/hw/arm/mps2-tz.c b/hw/arm/mps2-tz.c
index 394192b9b2..284c09c91d 100644
--- a/hw/arm/mps2-tz.c
+++ b/hw/arm/mps2-tz.c
@@ -1239,7 +1239,7 @@ static void mps2_set_remap(Object *obj, const char *value, Error **errp)
     }
 }
 
-static void mps2_machine_reset(MachineState *machine)
+static void mps2_machine_reset(MachineState *machine, ShutdownCause reason)
 {
     MPS2TZMachineState *mms = MPS2TZ_MACHINE(machine);
 
@@ -1249,7 +1249,7 @@ static void mps2_machine_reset(MachineState *machine)
      * reset see the correct mapping.
      */
     remap_memory(mms, mms->remap);
-    qemu_devices_reset();
+    qemu_devices_reset(reason);
 }
 
 static void mps2tz_class_init(ObjectClass *oc, void *data)
diff --git a/hw/core/reset.c b/hw/core/reset.c
index 36be82c491..bcf323d6dd 100644
--- a/hw/core/reset.c
+++ b/hw/core/reset.c
@@ -33,6 +33,7 @@ typedef struct QEMUResetEntry {
     QTAILQ_ENTRY(QEMUResetEntry) entry;
     QEMUResetHandler *func;
     void *opaque;
+    bool skip_on_snapshot_load;
 } QEMUResetEntry;
 
 static QTAILQ_HEAD(, QEMUResetEntry) reset_handlers =
@@ -47,6 +48,16 @@ void qemu_register_reset(QEMUResetHandler *func, void *opaque)
     QTAILQ_INSERT_TAIL(&reset_handlers, re, entry);
 }
 
+void qemu_register_reset_nosnapshotload(QEMUResetHandler *func, void *opaque)
+{
+    QEMUResetEntry *re = g_new0(QEMUResetEntry, 1);
+
+    re->func = func;
+    re->opaque = opaque;
+    re->skip_on_snapshot_load = true;
+    QTAILQ_INSERT_TAIL(&reset_handlers, re, entry);
+}
+
 void qemu_unregister_reset(QEMUResetHandler *func, void *opaque)
 {
     QEMUResetEntry *re;
@@ -60,12 +71,14 @@ void qemu_unregister_reset(QEMUResetHandler *func, void *opaque)
     }
 }
 
-void qemu_devices_reset(void)
+void qemu_devices_reset(ShutdownCause reason)
 {
     QEMUResetEntry *re, *nre;
 
     /* reset all devices */
     QTAILQ_FOREACH_SAFE(re, &reset_handlers, entry, nre) {
+        if (reason == SHUTDOWN_CAUSE_SNAPSHOT_LOAD && re->skip_on_snapshot_load)
+            continue;
         re->func(re->opaque);
     }
 }
diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c
index e53d5f0fa7..19ea7c2c66 100644
--- a/hw/hppa/machine.c
+++ b/hw/hppa/machine.c
@@ -411,12 +411,12 @@ static void machine_hppa_init(MachineState *machine)
     cpu[0]->env.gr[19] = FW_CFG_IO_BASE;
 }
 
-static void hppa_machine_reset(MachineState *ms)
+static void hppa_machine_reset(MachineState *ms, ShutdownCause reason)
 {
     unsigned int smp_cpus = ms->smp.cpus;
     int i;
 
-    qemu_devices_reset();
+    qemu_devices_reset(reason);
 
     /* Start all CPUs at the firmware entry point.
      *  Monarch CPU will initialize firmware, secondary CPUs
diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c
index 7fe8cce03e..860cfa00f5 100644
--- a/hw/i386/microvm.c
+++ b/hw/i386/microvm.c
@@ -467,7 +467,7 @@ static void microvm_machine_state_init(MachineState *machine)
     microvm_devices_init(mms);
 }
 
-static void microvm_machine_reset(MachineState *machine)
+static void microvm_machine_reset(MachineState *machine, ShutdownCause reason)
 {
     MicrovmMachineState *mms = MICROVM_MACHINE(machine);
     CPUState *cs;
@@ -480,7 +480,7 @@ static void microvm_machine_reset(MachineState *machine)
         mms->kernel_cmdline_fixed = true;
     }
 
-    qemu_devices_reset();
+    qemu_devices_reset(reason);
 
     CPU_FOREACH(cs) {
         cpu = X86_CPU(cs);
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 566accf7e6..66a0245a65 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1846,12 +1846,12 @@ static void pc_machine_initfn(Object *obj)
     cxl_machine_init(obj, &pcms->cxl_devices_state);
 }
 
-static void pc_machine_reset(MachineState *machine)
+static void pc_machine_reset(MachineState *machine, ShutdownCause reason)
 {
     CPUState *cs;
     X86CPU *cpu;
 
-    qemu_devices_reset();
+    qemu_devices_reset(reason);
 
     /* Reset APIC after devices have been reset to cancel
      * any changes that qemu_devices_reset() might have done.
@@ -1868,7 +1868,7 @@ static void pc_machine_reset(MachineState *machine)
 static void pc_machine_wakeup(MachineState *machine)
 {
     cpu_synchronize_all_states();
-    pc_machine_reset(machine);
+    pc_machine_reset(machine, SHUTDOWN_CAUSE_NONE);
     cpu_synchronize_all_post_reset();
 }
 
diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
index 61f4263953..919d5008ea 100644
--- a/hw/ppc/pegasos2.c
+++ b/hw/ppc/pegasos2.c
@@ -248,14 +248,14 @@ static void pegasos2_pci_config_write(Pegasos2MachineState *pm, int bus,
     pegasos2_mv_reg_write(pm, pcicfg + 4, len, val);
 }
 
-static void pegasos2_machine_reset(MachineState *machine)
+static void pegasos2_machine_reset(MachineState *machine, ShutdownCause reason)
 {
     Pegasos2MachineState *pm = PEGASOS2_MACHINE(machine);
     void *fdt;
     uint64_t d[2];
     int sz;
 
-    qemu_devices_reset();
+    qemu_devices_reset(reason);
     if (!pm->vof) {
         return; /* Firmware should set up machine so nothing to do */
     }
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 78e00afb9b..358a64b397 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -643,13 +643,13 @@ static void pnv_powerdown_notify(Notifier *n, void *opaque)
     }
 }
 
-static void pnv_reset(MachineState *machine)
+static void pnv_reset(MachineState *machine, ShutdownCause reason)
 {
     PnvMachineState *pnv = PNV_MACHINE(machine);
     IPMIBmc *bmc;
     void *fdt;
 
-    qemu_devices_reset();
+    qemu_devices_reset(reason);
 
     /*
      * The machine should provide by default an internal BMC simulator.
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 8bbaf4f8a4..6377e4cdb5 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1623,7 +1623,7 @@ void spapr_check_mmu_mode(bool guest_radix)
     }
 }
 
-static void spapr_machine_reset(MachineState *machine)
+static void spapr_machine_reset(MachineState *machine, ShutdownCause reason)
 {
     SpaprMachineState *spapr = SPAPR_MACHINE(machine);
     PowerPCCPU *first_ppc_cpu;
@@ -1649,7 +1649,7 @@ static void spapr_machine_reset(MachineState *machine)
         spapr_setup_hpt(spapr);
     }
 
-    qemu_devices_reset();
+    qemu_devices_reset(reason);
 
     spapr_ovec_cleanup(spapr->ov5_cas);
     spapr->ov5_cas = spapr_ovec_new();
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 03855c7231..8017acb1d5 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -405,7 +405,7 @@ static void s390_pv_prepare_reset(S390CcwMachineState *ms)
     s390_pv_prep_reset();
 }
 
-static void s390_machine_reset(MachineState *machine)
+static void s390_machine_reset(MachineState *machine, ShutdownCause reason)
 {
     S390CcwMachineState *ms = S390_CCW_MACHINE(machine);
     enum s390_reset reset_type;
@@ -427,7 +427,7 @@ static void s390_machine_reset(MachineState *machine)
             s390_machine_unprotect(ms);
         }
 
-        qemu_devices_reset();
+        qemu_devices_reset(reason);
         s390_crypto_reset();
 
         /* configure and start the ipl CPU only */
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 311ed17e18..90f1dd3aeb 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -231,7 +231,7 @@ struct MachineClass {
     const char *deprecation_reason;
 
     void (*init)(MachineState *state);
-    void (*reset)(MachineState *state);
+    void (*reset)(MachineState *state, ShutdownCause reason);
     void (*wakeup)(MachineState *state);
     int (*kvm_type)(MachineState *machine, const char *arg);
 
diff --git a/include/sysemu/reset.h b/include/sysemu/reset.h
index 0b0d6d7598..609e4d50c2 100644
--- a/include/sysemu/reset.h
+++ b/include/sysemu/reset.h
@@ -1,10 +1,13 @@
 #ifndef QEMU_SYSEMU_RESET_H
 #define QEMU_SYSEMU_RESET_H
 
+#include "qapi/qapi-events-run-state.h"
+
 typedef void QEMUResetHandler(void *opaque);
 
 void qemu_register_reset(QEMUResetHandler *func, void *opaque);
+void qemu_register_reset_nosnapshotload(QEMUResetHandler *func, void *opaque);
 void qemu_unregister_reset(QEMUResetHandler *func, void *opaque);
-void qemu_devices_reset(void);
+void qemu_devices_reset(ShutdownCause reason);
 
 #endif
diff --git a/migration/savevm.c b/migration/savevm.c
index 48e85c052c..a0cdb714f7 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -3058,7 +3058,7 @@ bool load_snapshot(const char *name, const char *vmstate,
         goto err_drain;
     }
 
-    qemu_system_reset(SHUTDOWN_CAUSE_NONE);
+    qemu_system_reset(SHUTDOWN_CAUSE_SNAPSHOT_LOAD);
     mis->from_src_file = f;
 
     if (!yank_register_instance(MIGRATION_YANK_INSTANCE, errp)) {
diff --git a/qapi/run-state.json b/qapi/run-state.json
index 9273ea6516..74ed0ac93c 100644
--- a/qapi/run-state.json
+++ b/qapi/run-state.json
@@ -86,12 +86,14 @@
 #                   ignores --no-reboot. This is useful for sanitizing
 #                   hypercalls on s390 that are used during kexec/kdump/boot
 #
+# @snapshot-load: A snapshot is being loaded by the record & replay subsystem.
+#
 ##
 { 'enum': 'ShutdownCause',
   # Beware, shutdown_caused_by_guest() depends on enumeration order
   'data': [ 'none', 'host-error', 'host-qmp-quit', 'host-qmp-system-reset',
             'host-signal', 'host-ui', 'guest-shutdown', 'guest-reset',
-            'guest-panic', 'subsystem-reset'] }
+            'guest-panic', 'subsystem-reset', 'snapshot-load'] }
 
 ##
 # @StatusInfo:
diff --git a/softmmu/runstate.c b/softmmu/runstate.c
index 1e68680b9d..03e1ee3a8a 100644
--- a/softmmu/runstate.c
+++ b/softmmu/runstate.c
@@ -441,9 +441,9 @@ void qemu_system_reset(ShutdownCause reason)
     cpu_synchronize_all_states();
 
     if (mc && mc->reset) {
-        mc->reset(current_machine);
+        mc->reset(current_machine, reason);
     } else {
-        qemu_devices_reset();
+        qemu_devices_reset(reason);
     }
     if (reason && reason != SHUTDOWN_CAUSE_SUBSYSTEM_RESET) {
         qapi_event_send_reset(shutdown_caused_by_guest(reason), reason);
-- 
2.37.3

Re: [PATCH v3 1/8] reset: allow registering handlers that aren't called by snapshot loading

[Peter Maydell]

(Cc'ing Markus for a QMP related question.)

On Fri, 14 Oct 2022 at 03:17, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> Snapshot loading only expects to call deterministic handlers, not
> non-deterministic ones. So introduce a way of registering handlers that
> won't be called when reseting for snapshots.
>
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>

> diff --git a/migration/savevm.c b/migration/savevm.c
> index 48e85c052c..a0cdb714f7 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -3058,7 +3058,7 @@ bool load_snapshot(const char *name, const char *vmstate,
>          goto err_drain;
>      }
>
> -    qemu_system_reset(SHUTDOWN_CAUSE_NONE);
> +    qemu_system_reset(SHUTDOWN_CAUSE_SNAPSHOT_LOAD);
>      mis->from_src_file = f;
>
>      if (!yank_register_instance(MIGRATION_YANK_INSTANCE, errp)) {
> diff --git a/qapi/run-state.json b/qapi/run-state.json
> index 9273ea6516..74ed0ac93c 100644
> --- a/qapi/run-state.json
> +++ b/qapi/run-state.json
> @@ -86,12 +86,14 @@
>  #                   ignores --no-reboot. This is useful for sanitizing
>  #                   hypercalls on s390 that are used during kexec/kdump/boot
>  #
> +# @snapshot-load: A snapshot is being loaded by the record & replay subsystem.
> +#
>  ##
>  { 'enum': 'ShutdownCause',
>    # Beware, shutdown_caused_by_guest() depends on enumeration order
>    'data': [ 'none', 'host-error', 'host-qmp-quit', 'host-qmp-system-reset',
>              'host-signal', 'host-ui', 'guest-shutdown', 'guest-reset',
> -            'guest-panic', 'subsystem-reset'] }
> +            'guest-panic', 'subsystem-reset', 'snapshot-load'] }

Markus: do we need to mark new enum values with some kind of "since n.n"
version info ?

>  ##
>  # @StatusInfo:
> diff --git a/softmmu/runstate.c b/softmmu/runstate.c
> index 1e68680b9d..03e1ee3a8a 100644
> --- a/softmmu/runstate.c
> +++ b/softmmu/runstate.c
> @@ -441,9 +441,9 @@ void qemu_system_reset(ShutdownCause reason)
>      cpu_synchronize_all_states();
>
>      if (mc && mc->reset) {
> -        mc->reset(current_machine);
> +        mc->reset(current_machine, reason);
>      } else {
> -        qemu_devices_reset();
> +        qemu_devices_reset(reason);
>      }
>      if (reason && reason != SHUTDOWN_CAUSE_SUBSYSTEM_RESET) {
>          qapi_event_send_reset(shutdown_caused_by_guest(reason), reason);

This change means that resets on snapshot-load, which previously
did not result in sending a QMP event, will now start to do so
with this new ShutdownCause type. I don't think we want that
change in behaviour.

(Also, as per the 'Beware' comment in run-state.json, we will
claim this to be a 'caused by guest' reset, which doesn't seem
right. If we suppress the sending the event that is moot, though.)

Markus: if we add a new value to the ShutdownCause enumeration,
how annoying is it if we decide we don't want it later? I guess
we can just leave it in the enum unused... (In this case we're
using it for purely internal purposes and it won't ever actually
wind up in any QMP events.)

thanks
-- PMM

Re: [PATCH v3 1/8] reset: allow registering handlers that aren't called by snapshot loading

[Jason A. Donenfeld]

On 10/24/22, Peter Maydell <peter.maydell@linaro.org> wrote:
> (Cc'ing Markus for a QMP related question.)
>
> On Fri, 14 Oct 2022 at 03:17, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>>
>> Snapshot loading only expects to call deterministic handlers, not
>> non-deterministic ones. So introduce a way of registering handlers that
>> won't be called when reseting for snapshots.
>>
>> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
>
>> diff --git a/migration/savevm.c b/migration/savevm.c
>> index 48e85c052c..a0cdb714f7 100644
>> --- a/migration/savevm.c
>> +++ b/migration/savevm.c
>> @@ -3058,7 +3058,7 @@ bool load_snapshot(const char *name, const char
>> *vmstate,
>>          goto err_drain;
>>      }
>>
>> -    qemu_system_reset(SHUTDOWN_CAUSE_NONE);
>> +    qemu_system_reset(SHUTDOWN_CAUSE_SNAPSHOT_LOAD);
>>      mis->from_src_file = f;
>>
>>      if (!yank_register_instance(MIGRATION_YANK_INSTANCE, errp)) {
>> diff --git a/qapi/run-state.json b/qapi/run-state.json
>> index 9273ea6516..74ed0ac93c 100644
>> --- a/qapi/run-state.json
>> +++ b/qapi/run-state.json
>> @@ -86,12 +86,14 @@
>>  #                   ignores --no-reboot. This is useful for sanitizing
>>  #                   hypercalls on s390 that are used during
>> kexec/kdump/boot
>>  #
>> +# @snapshot-load: A snapshot is being loaded by the record & replay
>> subsystem.
>> +#
>>  ##
>>  { 'enum': 'ShutdownCause',
>>    # Beware, shutdown_caused_by_guest() depends on enumeration order
>>    'data': [ 'none', 'host-error', 'host-qmp-quit',
>> 'host-qmp-system-reset',
>>              'host-signal', 'host-ui', 'guest-shutdown', 'guest-reset',
>> -            'guest-panic', 'subsystem-reset'] }
>> +            'guest-panic', 'subsystem-reset', 'snapshot-load'] }
>
> Markus: do we need to mark new enum values with some kind of "since n.n"
> version info ?
>
>>  ##
>>  # @StatusInfo:
>> diff --git a/softmmu/runstate.c b/softmmu/runstate.c
>> index 1e68680b9d..03e1ee3a8a 100644
>> --- a/softmmu/runstate.c
>> +++ b/softmmu/runstate.c
>> @@ -441,9 +441,9 @@ void qemu_system_reset(ShutdownCause reason)
>>      cpu_synchronize_all_states();
>>
>>      if (mc && mc->reset) {
>> -        mc->reset(current_machine);
>> +        mc->reset(current_machine, reason);
>>      } else {
>> -        qemu_devices_reset();
>> +        qemu_devices_reset(reason);
>>      }
>>      if (reason && reason != SHUTDOWN_CAUSE_SUBSYSTEM_RESET) {
>>          qapi_event_send_reset(shutdown_caused_by_guest(reason), reason);
>
> This change means that resets on snapshot-load, which previously
> did not result in sending a QMP event, will now start to do so
> with this new ShutdownCause type. I don't think we want that
> change in behaviour.

Good point. I'll exclude that case and send a v+1.

Jason

>
> (Also, as per the 'Beware' comment in run-state.json, we will
> claim this to be a 'caused by guest' reset, which doesn't seem
> right. If we suppress the sending the event that is moot, though.)
>
> Markus: if we add a new value to the ShutdownCause enumeration,
> how annoying is it if we decide we don't want it later? I guess
> we can just leave it in the enum unused... (In this case we're
> using it for purely internal purposes and it won't ever actually
> wind up in any QMP events.)
>
> thanks
> -- PMM
>


-- 
Jason A. Donenfeld
Deep Space Explorer
fr: +33 6 51 90 82 66
us: +1 513 476 1200
www.jasondonenfeld.com
www.zx2c4.com
zx2c4.com/keys/AB9942E6D4A4CFC3412620A749FC7012A5DE03AE.asc

Re: [PATCH v3 1/8] reset: allow registering handlers that aren't called by snapshot loading

[Markus Armbruster]

Peter Maydell <peter.maydell@linaro.org> writes:

> (Cc'ing Markus for a QMP related question.)
>
> On Fri, 14 Oct 2022 at 03:17, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>>
>> Snapshot loading only expects to call deterministic handlers, not
>> non-deterministic ones. So introduce a way of registering handlers that
>> won't be called when reseting for snapshots.
>>
>> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
>
>> diff --git a/migration/savevm.c b/migration/savevm.c
>> index 48e85c052c..a0cdb714f7 100644
>> --- a/migration/savevm.c
>> +++ b/migration/savevm.c
>> @@ -3058,7 +3058,7 @@ bool load_snapshot(const char *name, const char *vmstate,
>>          goto err_drain;
>>      }
>>
>> -    qemu_system_reset(SHUTDOWN_CAUSE_NONE);
>> +    qemu_system_reset(SHUTDOWN_CAUSE_SNAPSHOT_LOAD);
>>      mis->from_src_file = f;
>>
>>      if (!yank_register_instance(MIGRATION_YANK_INSTANCE, errp)) {
>> diff --git a/qapi/run-state.json b/qapi/run-state.json
>> index 9273ea6516..74ed0ac93c 100644
>> --- a/qapi/run-state.json
>> +++ b/qapi/run-state.json
>> @@ -86,12 +86,14 @@
>>  #                   ignores --no-reboot. This is useful for sanitizing
>>  #                   hypercalls on s390 that are used during kexec/kdump/boot
>>  #
>> +# @snapshot-load: A snapshot is being loaded by the record & replay subsystem.
>> +#
>>  ##
>>  { 'enum': 'ShutdownCause',
>>    # Beware, shutdown_caused_by_guest() depends on enumeration order
>>    'data': [ 'none', 'host-error', 'host-qmp-quit', 'host-qmp-system-reset',
>>              'host-signal', 'host-ui', 'guest-shutdown', 'guest-reset',
>> -            'guest-panic', 'subsystem-reset'] }
>> +            'guest-panic', 'subsystem-reset', 'snapshot-load'] }
>
> Markus: do we need to mark new enum values with some kind of "since n.n"
> version info ?

We do!  Commonly like

    # @snapshot-load: A snapshot is being loaded by the record & replay
    #                 subsystem (since 7.2)

>>  ##
>>  # @StatusInfo:
>> diff --git a/softmmu/runstate.c b/softmmu/runstate.c
>> index 1e68680b9d..03e1ee3a8a 100644
>> --- a/softmmu/runstate.c
>> +++ b/softmmu/runstate.c
>> @@ -441,9 +441,9 @@ void qemu_system_reset(ShutdownCause reason)
>>      cpu_synchronize_all_states();
>>
>>      if (mc && mc->reset) {
>> -        mc->reset(current_machine);
>> +        mc->reset(current_machine, reason);
>>      } else {
>> -        qemu_devices_reset();
>> +        qemu_devices_reset(reason);
>>      }
>>      if (reason && reason != SHUTDOWN_CAUSE_SUBSYSTEM_RESET) {
>>          qapi_event_send_reset(shutdown_caused_by_guest(reason), reason);
>
> This change means that resets on snapshot-load, which previously
> did not result in sending a QMP event, will now start to do so
> with this new ShutdownCause type. I don't think we want that
> change in behaviour.
>
> (Also, as per the 'Beware' comment in run-state.json, we will
> claim this to be a 'caused by guest' reset, which doesn't seem
> right. If we suppress the sending the event that is moot, though.)
>
> Markus: if we add a new value to the ShutdownCause enumeration,
> how annoying is it if we decide we don't want it later? I guess
> we can just leave it in the enum unused... (In this case we're
> using it for purely internal purposes and it won't ever actually
> wind up in any QMP events.)

Deleting enumeration values is a compatibility issue only if the value
is usable in QMP input.

"Purely internal" means it cannot occur in QMP output, and any attempt
to use it in input fails.  Aside: feels a bit fragile.

Having an enumeration type where some values are like this is mildly
ugly, because introspection still shows the value.

If we care about fragile or mildly ugly, we can mark such values with a
special feature flag, and have the QAPI generator keep them internal
(input, output, introspection).

Does this answer your question?

Re: [PATCH v3 1/8] reset: allow registering handlers that aren't called by snapshot loading

[Peter Maydell]

On Mon, 24 Oct 2022 at 13:28, Markus Armbruster <armbru@redhat.com> wrote:
>
> Peter Maydell <peter.maydell@linaro.org> writes:
> > Markus: if we add a new value to the ShutdownCause enumeration,
> > how annoying is it if we decide we don't want it later? I guess
> > we can just leave it in the enum unused... (In this case we're
> > using it for purely internal purposes and it won't ever actually
> > wind up in any QMP events.)
>
> Deleting enumeration values is a compatibility issue only if the value
> is usable in QMP input.
>
> "Purely internal" means it cannot occur in QMP output, and any attempt
> to use it in input fails.  Aside: feels a bit fragile.

In this case there are as far as I can see no QMP input commands
which use the enum at all -- it's only used in events, which are
always output, I think.

thanks
-- PMM

Re: [PATCH v3 1/8] reset: allow registering handlers that aren't called by snapshot loading

[Markus Armbruster]

Peter Maydell <peter.maydell@linaro.org> writes:

> On Mon, 24 Oct 2022 at 13:28, Markus Armbruster <armbru@redhat.com> wrote:
>>
>> Peter Maydell <peter.maydell@linaro.org> writes:
>> > Markus: if we add a new value to the ShutdownCause enumeration,
>> > how annoying is it if we decide we don't want it later? I guess
>> > we can just leave it in the enum unused... (In this case we're
>> > using it for purely internal purposes and it won't ever actually
>> > wind up in any QMP events.)
>>
>> Deleting enumeration values is a compatibility issue only if the value
>> is usable in QMP input.
>>
>> "Purely internal" means it cannot occur in QMP output, and any attempt
>> to use it in input fails.  Aside: feels a bit fragile.
>
> In this case there are as far as I can see no QMP input commands
> which use the enum at all -- it's only used in events, which are
> always output, I think.

They are.

Ascertaining "not used in QMP input" is pretty easy, and "not used in
CLI" isn't hard.  My point is that uses could creep in later.  This is
what makes "purely internal" fragile.

Re: [PATCH v3 1/8] reset: allow registering handlers that aren't called by snapshot loading

[Peter Maydell]

On Mon, 24 Oct 2022 at 14:20, Markus Armbruster <armbru@redhat.com> wrote:
>
> Peter Maydell <peter.maydell@linaro.org> writes:
>
> > On Mon, 24 Oct 2022 at 13:28, Markus Armbruster <armbru@redhat.com> wrote:
> >>
> >> Peter Maydell <peter.maydell@linaro.org> writes:
> >> > Markus: if we add a new value to the ShutdownCause enumeration,
> >> > how annoying is it if we decide we don't want it later? I guess
> >> > we can just leave it in the enum unused... (In this case we're
> >> > using it for purely internal purposes and it won't ever actually
> >> > wind up in any QMP events.)
> >>
> >> Deleting enumeration values is a compatibility issue only if the value
> >> is usable in QMP input.
> >>
> >> "Purely internal" means it cannot occur in QMP output, and any attempt
> >> to use it in input fails.  Aside: feels a bit fragile.
> >
> > In this case there are as far as I can see no QMP input commands
> > which use the enum at all -- it's only used in events, which are
> > always output, I think.
>
> They are.
>
> Ascertaining "not used in QMP input" is pretty easy, and "not used in
> CLI" isn't hard.  My point is that uses could creep in later.  This is
> what makes "purely internal" fragile.

True. But otoh if there's a meaningful use of the enum constant in
input in future we'll want to keep it around anyway.

I guess what I'm asking is: do you specifically want this patch
done some other way, or to require that "mark some values as
internal-only" feature in the QAPI generator, or are you OK with
it as-is?  QMP/QAPI is your area, so your call...

-- PMM

Re: [PATCH v3 1/8] reset: allow registering handlers that aren't called by snapshot loading

[Markus Armbruster]

Peter Maydell <peter.maydell@linaro.org> writes:

> On Mon, 24 Oct 2022 at 14:20, Markus Armbruster <armbru@redhat.com> wrote:
>>
>> Peter Maydell <peter.maydell@linaro.org> writes:
>>
>> > On Mon, 24 Oct 2022 at 13:28, Markus Armbruster <armbru@redhat.com> wrote:
>> >>
>> >> Peter Maydell <peter.maydell@linaro.org> writes:
>> >> > Markus: if we add a new value to the ShutdownCause enumeration,
>> >> > how annoying is it if we decide we don't want it later? I guess
>> >> > we can just leave it in the enum unused... (In this case we're
>> >> > using it for purely internal purposes and it won't ever actually
>> >> > wind up in any QMP events.)
>> >>
>> >> Deleting enumeration values is a compatibility issue only if the value
>> >> is usable in QMP input.
>> >>
>> >> "Purely internal" means it cannot occur in QMP output, and any attempt
>> >> to use it in input fails.  Aside: feels a bit fragile.
>> >
>> > In this case there are as far as I can see no QMP input commands
>> > which use the enum at all -- it's only used in events, which are
>> > always output, I think.
>>
>> They are.
>>
>> Ascertaining "not used in QMP input" is pretty easy, and "not used in
>> CLI" isn't hard.  My point is that uses could creep in later.  This is
>> what makes "purely internal" fragile.
>
> True. But otoh if there's a meaningful use of the enum constant in
> input in future we'll want to keep it around anyway.
>
> I guess what I'm asking is: do you specifically want this patch
> done some other way, or to require that "mark some values as
> internal-only" feature in the QAPI generator, or are you OK with
> it as-is?  QMP/QAPI is your area, so your call...

QAPI was designed to specify QMP.  We pretty soon discovered that QAPI
types can be useful elsewhere, and added some to the schema without
marking them.  Introspection doesn't show these.  Generated
documentation does.  Known shortcoming of the doc generator.

This case differs in that we're adding an internal-only member to a type
that is used by QMP.  Both introspection and documentation will show it.

Interface introspection and (especially!) documentation showing stuff
that doesn't exist in the interface is certainly less than ideal.
However, I don't want to hold this patch hostage to getting QAPI
shortcomings addressed.

Instead, I want QMP documentation to make clear that this value cannot
actually occur.

Fair?

Re: [PATCH v3 1/8] reset: allow registering handlers that aren't called by snapshot loading

[Jason A. Donenfeld]

On Mon, Oct 24, 2022 at 7:40 PM Markus Armbruster <armbru@redhat.com> wrote:
>
> Peter Maydell <peter.maydell@linaro.org> writes:
>
> > On Mon, 24 Oct 2022 at 14:20, Markus Armbruster <armbru@redhat.com> wrote:
> >>
> >> Peter Maydell <peter.maydell@linaro.org> writes:
> >>
> >> > On Mon, 24 Oct 2022 at 13:28, Markus Armbruster <armbru@redhat.com> wrote:
> >> >>
> >> >> Peter Maydell <peter.maydell@linaro.org> writes:
> >> >> > Markus: if we add a new value to the ShutdownCause enumeration,
> >> >> > how annoying is it if we decide we don't want it later? I guess
> >> >> > we can just leave it in the enum unused... (In this case we're
> >> >> > using it for purely internal purposes and it won't ever actually
> >> >> > wind up in any QMP events.)
> >> >>
> >> >> Deleting enumeration values is a compatibility issue only if the value
> >> >> is usable in QMP input.
> >> >>
> >> >> "Purely internal" means it cannot occur in QMP output, and any attempt
> >> >> to use it in input fails.  Aside: feels a bit fragile.
> >> >
> >> > In this case there are as far as I can see no QMP input commands
> >> > which use the enum at all -- it's only used in events, which are
> >> > always output, I think.
> >>
> >> They are.
> >>
> >> Ascertaining "not used in QMP input" is pretty easy, and "not used in
> >> CLI" isn't hard.  My point is that uses could creep in later.  This is
> >> what makes "purely internal" fragile.
> >
> > True. But otoh if there's a meaningful use of the enum constant in
> > input in future we'll want to keep it around anyway.
> >
> > I guess what I'm asking is: do you specifically want this patch
> > done some other way, or to require that "mark some values as
> > internal-only" feature in the QAPI generator, or are you OK with
> > it as-is?  QMP/QAPI is your area, so your call...
>
> QAPI was designed to specify QMP.  We pretty soon discovered that QAPI
> types can be useful elsewhere, and added some to the schema without
> marking them.  Introspection doesn't show these.  Generated
> documentation does.  Known shortcoming of the doc generator.
>
> This case differs in that we're adding an internal-only member to a type
> that is used by QMP.  Both introspection and documentation will show it.
>
> Interface introspection and (especially!) documentation showing stuff
> that doesn't exist in the interface is certainly less than ideal.
> However, I don't want to hold this patch hostage to getting QAPI
> shortcomings addressed.
>
> Instead, I want QMP documentation to make clear that this value cannot
> actually occur.
>
> Fair?

Made mention of it in v4, just posted.

https://lore.kernel.org/all/20221025004327.568476-2-Jason@zx2c4.com/#Z31qapi:run-state.json

[PATCH v3 2/8] x86: do not re-randomize RNG seed on snapshot load

[Jason A. Donenfeld]

Snapshot loading is supposed to be deterministic, so we shouldn't
re-randomize the various seeds used.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 hw/i386/x86.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index 1148f70c03..bd50a064a3 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -1111,7 +1111,7 @@ void x86_load_linux(X86MachineState *x86ms,
         setup_data->type = cpu_to_le32(SETUP_RNG_SEED);
         setup_data->len = cpu_to_le32(RNG_SEED_LENGTH);
         qemu_guest_getrandom_nofail(setup_data->data, RNG_SEED_LENGTH);
-        qemu_register_reset(reset_rng_seed, setup_data);
+        qemu_register_reset_nosnapshotload(reset_rng_seed, setup_data);
         fw_cfg_add_bytes_callback(fw_cfg, FW_CFG_KERNEL_DATA, reset_rng_seed, NULL,
                                   setup_data, kernel, kernel_size, true);
     } else {
-- 
2.37.3

[PATCH v3 3/8] device-tree: add re-randomization helper function

[Jason A. Donenfeld]

When the system reboots, the rng-seed that the FDT has should be
re-randomized, so that the new boot gets a new seed. Several
architectures require this functionality, so export a function for
injecting a new seed into the given FDT.

Cc: Alistair Francis <alistair.francis@wdc.com>
Cc: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 include/sysemu/device_tree.h |  9 +++++++++
 softmmu/device_tree.c        | 21 +++++++++++++++++++++
 2 files changed, 30 insertions(+)

diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h
index ef060a9759..d552f324b6 100644
--- a/include/sysemu/device_tree.h
+++ b/include/sysemu/device_tree.h
@@ -196,6 +196,15 @@ int qemu_fdt_setprop_sized_cells_from_array(void *fdt,
                                                 qdt_tmp);                 \
     })
 
+
+/**
+ * qemu_fdt_randomize_seeds:
+ * @fdt: device tree blob
+ *
+ * Re-randomize all "rng-seed" properties with new seeds.
+ */
+void qemu_fdt_randomize_seeds(void *fdt);
+
 #define FDT_PCI_RANGE_RELOCATABLE          0x80000000
 #define FDT_PCI_RANGE_PREFETCHABLE         0x40000000
 #define FDT_PCI_RANGE_ALIASED              0x20000000
diff --git a/softmmu/device_tree.c b/softmmu/device_tree.c
index 6ca3fad285..d986c7b7b3 100644
--- a/softmmu/device_tree.c
+++ b/softmmu/device_tree.c
@@ -22,6 +22,7 @@
 #include "qemu/option.h"
 #include "qemu/bswap.h"
 #include "qemu/cutils.h"
+#include "qemu/guest-random.h"
 #include "sysemu/device_tree.h"
 #include "hw/loader.h"
 #include "hw/boards.h"
@@ -643,3 +644,23 @@ out:
     g_free(propcells);
     return ret;
 }
+
+void qemu_fdt_randomize_seeds(void *fdt)
+{
+    int noffset, poffset, len;
+    const char *name;
+    uint8_t *data;
+
+    for (noffset = fdt_next_node(fdt, 0, NULL);
+         noffset >= 0;
+         noffset = fdt_next_node(fdt, noffset, NULL)) {
+        for (poffset = fdt_first_property_offset(fdt, noffset);
+             poffset >= 0;
+             poffset = fdt_next_property_offset(fdt, poffset)) {
+            data = (uint8_t *)fdt_getprop_by_offset(fdt, poffset, &name, &len);
+            if (!data || strcmp(name, "rng-seed"))
+                continue;
+            qemu_guest_getrandom_nofail(data, len);
+        }
+    }
+}
-- 
2.37.3

[PATCH v3 4/8] arm: re-randomize rng-seed on reboot

[Jason A. Donenfeld]

When the system reboots, the rng-seed that the FDT has should be
re-randomized, so that the new boot gets a new seed. Since the FDT is in
the ROM region at this point, we add a hook right after the ROM has been
added, so that we have a pointer to that copy of the FDT.

Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: qemu-arm@nongnu.org
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 hw/arm/boot.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index ada2717f76..511f7b22b1 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -683,6 +683,8 @@ int arm_load_dtb(hwaddr addr, const struct arm_boot_info *binfo,
      * the DTB is copied again upon reset, even if addr points into RAM.
      */
     rom_add_blob_fixed_as("dtb", fdt, size, addr, as);
+    qemu_register_reset_nosnapshotload(qemu_fdt_randomize_seeds,
+                                       rom_ptr_for_as(as, addr, size));
 
     g_free(fdt);
 
-- 
2.37.3

[PATCH v3 5/8] riscv: re-randomize rng-seed on reboot

[Jason A. Donenfeld]

When the system reboots, the rng-seed that the FDT has should be
re-randomized, so that the new boot gets a new seed. Since the FDT is in
the ROM region at this point, we add a hook right after the ROM has been
added, so that we have a pointer to that copy of the FDT.

Cc: Palmer Dabbelt <palmer@dabbelt.com>
Cc: Alistair Francis <alistair.francis@wdc.com>
Cc: Bin Meng <bin.meng@windriver.com>
Cc: qemu-riscv@nongnu.org
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 hw/riscv/boot.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
index 1ae7596873..c389edb3cd 100644
--- a/hw/riscv/boot.c
+++ b/hw/riscv/boot.c
@@ -30,6 +30,7 @@
 #include "sysemu/device_tree.h"
 #include "sysemu/qtest.h"
 #include "sysemu/kvm.h"
+#include "sysemu/reset.h"
 
 #include <libfdt.h>
 
@@ -241,6 +242,8 @@ uint64_t riscv_load_fdt(hwaddr dram_base, uint64_t mem_size, void *fdt)
 
     rom_add_blob_fixed_as("fdt", fdt, fdtsize, fdt_addr,
                           &address_space_memory);
+    qemu_register_reset_nosnapshotload(qemu_fdt_randomize_seeds,
+                        rom_ptr_for_as(&address_space_memory, fdt_addr, fdtsize));
 
     return fdt_addr;
 }
-- 
2.37.3

[PATCH v3 6/8] openrisc: re-randomize rng-seed on reboot

[Jason A. Donenfeld]

When the system reboots, the rng-seed that the FDT has should be
re-randomized, so that the new boot gets a new seed. Since the FDT is in
the ROM region at this point, we add a hook right after the ROM has been
added, so that we have a pointer to that copy of the FDT.

Cc: Stafford Horne <shorne@gmail.com>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 hw/openrisc/boot.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/openrisc/boot.c b/hw/openrisc/boot.c
index 128ccbcba2..007e80cd5a 100644
--- a/hw/openrisc/boot.c
+++ b/hw/openrisc/boot.c
@@ -14,6 +14,7 @@
 #include "hw/openrisc/boot.h"
 #include "sysemu/device_tree.h"
 #include "sysemu/qtest.h"
+#include "sysemu/reset.h"
 
 #include <libfdt.h>
 
@@ -111,6 +112,8 @@ uint32_t openrisc_load_fdt(void *fdt, hwaddr load_start,
 
     rom_add_blob_fixed_as("fdt", fdt, fdtsize, fdt_addr,
                           &address_space_memory);
+    qemu_register_reset_nosnapshotload(qemu_fdt_randomize_seeds,
+                        rom_ptr_for_as(&address_space_memory, fdt_addr, fdtsize));
 
     return fdt_addr;
 }
-- 
2.37.3

Re: [PATCH v3 6/8] openrisc: re-randomize rng-seed on reboot

[Stafford Horne]

Hi Jason,

On Thu, Oct 13, 2022 at 08:16:51PM -0600, Jason A. Donenfeld wrote:
> When the system reboots, the rng-seed that the FDT has should be
> re-randomized, so that the new boot gets a new seed. Since the FDT is in
> the ROM region at this point, we add a hook right after the ROM has been
> added, so that we have a pointer to that copy of the FDT.

This looks good to me.

Acked-by: Stafford Horne <shorne@gmail.com>

> Cc: Stafford Horne <shorne@gmail.com>
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> ---
>  hw/openrisc/boot.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/hw/openrisc/boot.c b/hw/openrisc/boot.c
> index 128ccbcba2..007e80cd5a 100644
> --- a/hw/openrisc/boot.c
> +++ b/hw/openrisc/boot.c
> @@ -14,6 +14,7 @@
>  #include "hw/openrisc/boot.h"
>  #include "sysemu/device_tree.h"
>  #include "sysemu/qtest.h"
> +#include "sysemu/reset.h"
>  
>  #include <libfdt.h>
>  
> @@ -111,6 +112,8 @@ uint32_t openrisc_load_fdt(void *fdt, hwaddr load_start,
>  
>      rom_add_blob_fixed_as("fdt", fdt, fdtsize, fdt_addr,
>                            &address_space_memory);
> +    qemu_register_reset_nosnapshotload(qemu_fdt_randomize_seeds,
> +                        rom_ptr_for_as(&address_space_memory, fdt_addr, fdtsize));
>  
>      return fdt_addr;
>  }
> -- 
> 2.37.3
> 

[PATCH v3 7/8] rx: re-randomize rng-seed on reboot

[Jason A. Donenfeld]

When the system reboots, the rng-seed that the FDT has should be
re-randomized, so that the new boot gets a new seed. Since the FDT is in
the ROM region at this point, we add a hook right after the ROM has been
added, so that we have a pointer to that copy of the FDT.

Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 hw/rx/rx-gdbsim.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/rx/rx-gdbsim.c b/hw/rx/rx-gdbsim.c
index 8ffe1b8035..47c17026c7 100644
--- a/hw/rx/rx-gdbsim.c
+++ b/hw/rx/rx-gdbsim.c
@@ -25,6 +25,7 @@
 #include "hw/rx/rx62n.h"
 #include "sysemu/qtest.h"
 #include "sysemu/device_tree.h"
+#include "sysemu/reset.h"
 #include "hw/boards.h"
 #include "qom/object.h"
 
@@ -148,6 +149,8 @@ static void rx_gdbsim_init(MachineState *machine)
             dtb_offset = ROUND_DOWN(machine->ram_size - dtb_size, 16);
             rom_add_blob_fixed("dtb", dtb, dtb_size,
                                SDRAM_BASE + dtb_offset);
+            qemu_register_reset_nosnapshotload(qemu_fdt_randomize_seeds,
+                                rom_ptr(SDRAM_BASE + dtb_offset, dtb_size));
             /* Set dtb address to R1 */
             RX_CPU(first_cpu)->env.regs[1] = SDRAM_BASE + dtb_offset;
         }
-- 
2.37.3

[PATCH v3 8/8] mips: re-randomize rng-seed on reboot

[Jason A. Donenfeld]

When the system reboots, the rng-seed that the FDT has should be
re-randomized, so that the new boot gets a new seed. Since the FDT is in
the ROM region at this point, we add a hook right after the ROM has been
added, so that we have a pointer to that copy of the FDT.

Cc: Aleksandar Rikalo <aleksandar.rikalo@syrmia.com>
Cc: Paul Burton <paulburton@kernel.org>
Cc: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 hw/mips/boston.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/mips/boston.c b/hw/mips/boston.c
index d2ab9da1a0..cab63f43bf 100644
--- a/hw/mips/boston.c
+++ b/hw/mips/boston.c
@@ -41,6 +41,7 @@
 #include "sysemu/sysemu.h"
 #include "sysemu/qtest.h"
 #include "sysemu/runstate.h"
+#include "sysemu/reset.h"
 
 #include <libfdt.h>
 #include "qom/object.h"
@@ -810,6 +811,8 @@ static void boston_mach_init(MachineState *machine)
             /* Calculate real fdt size after filter */
             dt_size = fdt_totalsize(dtb_load_data);
             rom_add_blob_fixed("dtb", dtb_load_data, dt_size, dtb_paddr);
+            qemu_register_reset_nosnapshotload(qemu_fdt_randomize_seeds,
+                                rom_ptr(dtb_paddr, dt_size));
         } else {
             /* Try to load file as FIT */
             fit_err = load_fit(&boston_fit_loader, machine->kernel_filename, s);
-- 
2.37.3

Re: [PATCH v3 0/8] rerandomize RNG seeds on reboot and handle record&replay

[Jason A. Donenfeld]

Hey Peter,

Could you queue this up again like last time?

Thanks,
Jason