All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] hw/riscv: virt: pass random seed to fdt
@ 2022-06-13 11:58 Jason A. Donenfeld
  2022-06-15  4:05 ` Bin Meng
  2022-06-29  2:09 ` Alistair Francis
  0 siblings, 2 replies; 16+ messages in thread
From: Jason A. Donenfeld @ 2022-06-13 11:58 UTC (permalink / raw)
  To: qemu-devel; +Cc: Jason A. Donenfeld, Alistair Francis

If the FDT contains /chosen/rng-seed, then the Linux RNG will use it to
initialize early. Set this using the usual guest random number
generation function. This is confirmed to successfully initialize the
RNG on Linux 5.19-rc2.

Cc: Alistair Francis <alistair.francis@wdc.com>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 hw/riscv/virt.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index bc424dd2f5..368a723bf6 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -21,6 +21,7 @@
 #include "qemu/osdep.h"
 #include "qemu/units.h"
 #include "qemu/error-report.h"
+#include "qemu/guest-random.h"
 #include "qapi/error.h"
 #include "hw/boards.h"
 #include "hw/loader.h"
@@ -998,6 +999,7 @@ static void create_fdt(RISCVVirtState *s, const MemMapEntry *memmap,
     MachineState *mc = MACHINE(s);
     uint32_t phandle = 1, irq_mmio_phandle = 1, msi_pcie_phandle = 1;
     uint32_t irq_pcie_phandle = 1, irq_virtio_phandle = 1;
+    uint8_t rng_seed[32];
 
     if (mc->dtb) {
         mc->fdt = load_device_tree(mc->dtb, &s->fdt_size);
@@ -1046,6 +1048,10 @@ update_bootargs:
     if (cmdline && *cmdline) {
         qemu_fdt_setprop_string(mc->fdt, "/chosen", "bootargs", cmdline);
     }
+
+    /* Pass seed to RNG. */
+    qemu_guest_getrandom_nofail(rng_seed, sizeof(rng_seed));
+    qemu_fdt_setprop(mc->fdt, "/chosen", "rng-seed", rng_seed, sizeof(rng_seed));
 }
 
 static inline DeviceState *gpex_pcie_init(MemoryRegion *sys_mem,
-- 
2.35.1



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH] hw/riscv: virt: pass random seed to fdt
  2022-06-13 11:58 [PATCH] hw/riscv: virt: pass random seed to fdt Jason A. Donenfeld
@ 2022-06-15  4:05 ` Bin Meng
  2022-06-16  2:32   ` Alistair Francis
  2022-06-29  2:09 ` Alistair Francis
  1 sibling, 1 reply; 16+ messages in thread
From: Bin Meng @ 2022-06-15  4:05 UTC (permalink / raw)
  To: Jason A. Donenfeld; +Cc: qemu-devel@nongnu.org Developers, Alistair Francis

On Mon, Jun 13, 2022 at 8:08 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> If the FDT contains /chosen/rng-seed, then the Linux RNG will use it to
> initialize early. Set this using the usual guest random number
> generation function. This is confirmed to successfully initialize the
> RNG on Linux 5.19-rc2.
>
> Cc: Alistair Francis <alistair.francis@wdc.com>
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> ---
>  hw/riscv/virt.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index bc424dd2f5..368a723bf6 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -21,6 +21,7 @@
>  #include "qemu/osdep.h"
>  #include "qemu/units.h"
>  #include "qemu/error-report.h"
> +#include "qemu/guest-random.h"
>  #include "qapi/error.h"
>  #include "hw/boards.h"
>  #include "hw/loader.h"
> @@ -998,6 +999,7 @@ static void create_fdt(RISCVVirtState *s, const MemMapEntry *memmap,
>      MachineState *mc = MACHINE(s);
>      uint32_t phandle = 1, irq_mmio_phandle = 1, msi_pcie_phandle = 1;
>      uint32_t irq_pcie_phandle = 1, irq_virtio_phandle = 1;
> +    uint8_t rng_seed[32];
>
>      if (mc->dtb) {
>          mc->fdt = load_device_tree(mc->dtb, &s->fdt_size);
> @@ -1046,6 +1048,10 @@ update_bootargs:
>      if (cmdline && *cmdline) {
>          qemu_fdt_setprop_string(mc->fdt, "/chosen", "bootargs", cmdline);
>      }
> +
> +    /* Pass seed to RNG. */

nits: please remove the ending period

> +    qemu_guest_getrandom_nofail(rng_seed, sizeof(rng_seed));
> +    qemu_fdt_setprop(mc->fdt, "/chosen", "rng-seed", rng_seed, sizeof(rng_seed));
>  }
>
>  static inline DeviceState *gpex_pcie_init(MemoryRegion *sys_mem,
> --

Otherwise,
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] hw/riscv: virt: pass random seed to fdt
  2022-06-15  4:05 ` Bin Meng
@ 2022-06-16  2:32   ` Alistair Francis
  2022-06-16 10:01     ` Jason A. Donenfeld
  0 siblings, 1 reply; 16+ messages in thread
From: Alistair Francis @ 2022-06-16  2:32 UTC (permalink / raw)
  To: Bin Meng
  Cc: Jason A. Donenfeld, qemu-devel@nongnu.org Developers, Alistair Francis

On Wed, Jun 15, 2022 at 2:07 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> On Mon, Jun 13, 2022 at 8:08 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> >
> > If the FDT contains /chosen/rng-seed, then the Linux RNG will use it to
> > initialize early. Set this using the usual guest random number
> > generation function. This is confirmed to successfully initialize the
> > RNG on Linux 5.19-rc2.
> >
> > Cc: Alistair Francis <alistair.francis@wdc.com>
> > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>

Thanks!

Applied to riscv-to-apply.next with the full stop removed

Alistair

> > ---
> >  hw/riscv/virt.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> > index bc424dd2f5..368a723bf6 100644
> > --- a/hw/riscv/virt.c
> > +++ b/hw/riscv/virt.c
> > @@ -21,6 +21,7 @@
> >  #include "qemu/osdep.h"
> >  #include "qemu/units.h"
> >  #include "qemu/error-report.h"
> > +#include "qemu/guest-random.h"
> >  #include "qapi/error.h"
> >  #include "hw/boards.h"
> >  #include "hw/loader.h"
> > @@ -998,6 +999,7 @@ static void create_fdt(RISCVVirtState *s, const MemMapEntry *memmap,
> >      MachineState *mc = MACHINE(s);
> >      uint32_t phandle = 1, irq_mmio_phandle = 1, msi_pcie_phandle = 1;
> >      uint32_t irq_pcie_phandle = 1, irq_virtio_phandle = 1;
> > +    uint8_t rng_seed[32];
> >
> >      if (mc->dtb) {
> >          mc->fdt = load_device_tree(mc->dtb, &s->fdt_size);
> > @@ -1046,6 +1048,10 @@ update_bootargs:
> >      if (cmdline && *cmdline) {
> >          qemu_fdt_setprop_string(mc->fdt, "/chosen", "bootargs", cmdline);
> >      }
> > +
> > +    /* Pass seed to RNG. */
>
> nits: please remove the ending period
>
> > +    qemu_guest_getrandom_nofail(rng_seed, sizeof(rng_seed));
> > +    qemu_fdt_setprop(mc->fdt, "/chosen", "rng-seed", rng_seed, sizeof(rng_seed));
> >  }
> >
> >  static inline DeviceState *gpex_pcie_init(MemoryRegion *sys_mem,
> > --
>
> Otherwise,
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
>


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] hw/riscv: virt: pass random seed to fdt
  2022-06-16  2:32   ` Alistair Francis
@ 2022-06-16 10:01     ` Jason A. Donenfeld
  2022-06-16 12:17       ` Alistair Francis
  0 siblings, 1 reply; 16+ messages in thread
From: Jason A. Donenfeld @ 2022-06-16 10:01 UTC (permalink / raw)
  To: Alistair Francis
  Cc: Bin Meng, qemu-devel@nongnu.org Developers, Alistair Francis

Hi Alistair,

On Thu, Jun 16, 2022 at 12:32:36PM +1000, Alistair Francis wrote:
> Applied to riscv-to-apply.next with the full stop removed

Great, thanks. Just wondering: am I looking in the right repo? I don't
see it here: https://github.com/alistair23/qemu/commits/riscv-to-apply.next

Jason


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] hw/riscv: virt: pass random seed to fdt
  2022-06-16 10:01     ` Jason A. Donenfeld
@ 2022-06-16 12:17       ` Alistair Francis
  0 siblings, 0 replies; 16+ messages in thread
From: Alistair Francis @ 2022-06-16 12:17 UTC (permalink / raw)
  To: Jason A. Donenfeld
  Cc: Bin Meng, qemu-devel@nongnu.org Developers, Alistair Francis

On Thu, Jun 16, 2022 at 8:01 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> Hi Alistair,
>
> On Thu, Jun 16, 2022 at 12:32:36PM +1000, Alistair Francis wrote:
> > Applied to riscv-to-apply.next with the full stop removed
>
> Great, thanks. Just wondering: am I looking in the right repo? I don't
> see it here: https://github.com/alistair23/qemu/commits/riscv-to-apply.next

That's the right repo, I just have to push the latest updates. You
should see it there tomorrow

Alistair

>
> Jason


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] hw/riscv: virt: pass random seed to fdt
  2022-06-13 11:58 [PATCH] hw/riscv: virt: pass random seed to fdt Jason A. Donenfeld
  2022-06-15  4:05 ` Bin Meng
@ 2022-06-29  2:09 ` Alistair Francis
  2022-07-05  1:09   ` Jason A. Donenfeld
  1 sibling, 1 reply; 16+ messages in thread
From: Alistair Francis @ 2022-06-29  2:09 UTC (permalink / raw)
  To: Jason A. Donenfeld; +Cc: qemu-devel@nongnu.org Developers, Alistair Francis

On Mon, Jun 13, 2022 at 10:10 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> If the FDT contains /chosen/rng-seed, then the Linux RNG will use it to
> initialize early. Set this using the usual guest random number
> generation function. This is confirmed to successfully initialize the
> RNG on Linux 5.19-rc2.

I have a Linux 5.8 test case that is failing due to this patch.

The command line is:

qemu-system-riscv64 \
-machine virt -m 64M \
-cpu rv64,mmu=false \
-serial mon:stdio -serial null -nographic \
-append "root=/dev/vda rw highres=off  console=ttyS0 mem=1G ip=dhcp
earlycon=sbi" \
-device virtio-net-device,netdev=net0,mac=52:54:00:12:34:02 \
-netdev user,id=net0 \
-object rng-random,filename=/dev/urandom,id=rng0 \
-device virtio-rng-device,rng=rng0 \
-smp 1  \
-d guest_errors \
-kernel ./images/qemuriscv64/nommu-Image \
-drive id=disk0,file=./images/qemuriscv64/nommu-rootfs.ext2,if=none,format=raw \
-device virtio -blk-device,drive=disk0 \
-bios none

The working log (before this commit) is:

[    0.000000] Linux version 5.8.0 (alistair@risc6-mainframe)
(riscv64-elf-gcc (Arch Linux Repositories) 10.1.0, GNU ld (GNU
Binutils) 2.34) #2 SMP Wed Sep
30 12:02:11 PDT 2020
[    0.000000] earlycon: uart8250 at MMIO 0x0000000010000000 (options
'115200n8')
[    0.000000] printk: bootconsole [uart8250] enabled
[    0.000000] Zone ranges:
[    0.000000]   DMA32    [mem 0x0000000080000000-0x0000000083ffffff]
[    0.000000]   Normal   empty
[    0.000000] Movable zone start for each node
[    0.000000] Early memory node ranges
[    0.000000]   node   0: [mem 0x0000000080000000-0x0000000083ffffff]
[    0.000000] Initmem setup node 0 [mem 0x0000000080000000-0x0000000083ffffff]
[    0.000000] riscv: ISA extensions abcdefhimnrs
[    0.000000] riscv: ELF capabilities acdfim
[    0.000000] percpu: max_distance=0xc000 too large for vmalloc space 0x0
[    0.000000] percpu: Embedded 12 pages/cpu s18528 r0 d30624 u49152
[    0.000000] Built 1 zonelists, mobility grouping on.  Total pages: 16160
[    0.000000] Kernel command line: root=/dev/vda rw
earlycon=uart8250,mmio,0x10000000,115200n8 console=ttyS0
[    0.000000] Dentry cache hash table entries: 8192 (order: 4, 65536
bytes, linear)
[    0.000000] Inode-cache hash table entries: 4096 (order: 3, 32768
bytes, linear)
[    0.000000] Sorting __ex_table...
[    0.000000] mem auto-init: stack:off, heap alloc:off, heap free:off
[    0.000000] Memory: 62472K/65536K available (1369K kernel code,
144K rwdata, 238K rodata, 106K init, 134K bss, 3064K reserved, 0K
cma-reserved)
[    0.000000] rcu: Hierarchical RCU implementation.
[    0.000000] rcu:     RCU restricting CPUs from NR_CPUS=8 to nr_cpu_ids=1.
[    0.000000] rcu: RCU calculated value of scheduler-enlistment delay
is 25 jiffies.
[    0.000000] rcu: Adjusting geometry for rcu_fanout_leaf=16, nr_cpu_ids=1
[    0.000000] NR_IRQS: 64, nr_irqs: 64, preallocated irqs: 0
[    0.000000] riscv-intc: 64 local interrupts mapped
sifive_plic_read: Invalid register read 0x200c
sifive_plic_write: Invalid enable write 0x200c
[    0.000000] plic: plic@c000000: mapped 96 interrupts with 1
handlers for 2 contexts.
[    0.000000] riscv_timer_init_dt: Registering clocksource cpuid [0] hartid [0]
[    0.000000] clocksource: riscv_clocksource: mask:
0xffffffffffffffff max_cycles: 0x24e6a1710, max_idle_ns: 440795202120
ns
[    0.000106] sched_clock: 64 bits at 10MHz, resolution 100ns, wraps
every 4398046511100ns
[    0.002649] Console: colour dummy device 80x25
[    0.003599] Calibrating delay loop (skipped), value calculated
using timer frequency.. 20.00 BogoMIPS (lpj=40000)
[    0.003960] pid_max: default: 4096 minimum: 301
[    0.004718] Mount-cache hash table entries: 512 (order: 0, 4096
bytes, linear)
[    0.004922] Mountpoint-cache hash table entries: 512 (order: 0,
4096 bytes, linear)
[    0.022781] rcu: Hierarchical SRCU implementation.
[    0.024374] smp: Bringing up secondary CPUs ...
[    0.024583] smp: Brought up 1 node, 1 CPU
[    0.030450] devtmpfs: initialized
[    0.035183] clocksource: jiffies: mask: 0xffffffff max_cycles:
0xffffffff, max_idle_ns: 7645041785100000 ns
[    0.035600] futex hash table entries: 16 (order: -2, 1024 bytes, linear)
[    0.055175] clocksource: Switched to clocksource riscv_clocksource
[    0.073226] workingset: timestamp_bits=62 max_order=14 bucket_order=0
[    0.078326] Serial: 8250/16550 driver, 1 ports, IRQ sharing disabled
[    0.082509] printk: console [ttyS0] disabled
[    0.083408] 10000000.uart: ttyS0 at MMIO 0x10000000 (irq = 2,
base_baud = 230400) is a 16550A
[    0.084805] printk: console [ttyS0] enabled
[    0.084805] printk: console [ttyS0] enabled
[    0.085242] printk: bootconsole [uart8250] disabled
[    0.085242] printk: bootconsole [uart8250] disabled
virtio_mmio_write: attempt to write guest features with
guest_features_sel > 0 in legacy mode
[    0.095810] virtio_blk virtio2: [vda] 122880 512-byte logical
blocks (62.9 MB/60.0 MiB)
[    0.096155] vda: detected capacity change from 0 to 62914560
[    0.099882] random: get_random_bytes called from 0x0000000080020c8c
with crng_init=0
[    0.120690] EXT4-fs (vda): warning: mounting unchecked fs, running
e2fsck is recommended
[    0.160540] EXT4-fs (vda): mounted filesystem without journal. Opts: (null)
[    0.160910] VFS: Mounted root (ext4 filesystem) on device 254:0.
[    0.162645] devtmpfs: mounted
[    0.171902] Freeing unused kernel memory: 104K
[    0.172061] This architecture does not have kernel memory protection.
[    0.172387] Run /sbin/init as init process
[    0.174104] Run /etc/init as init process
[    0.174534] Run /bin/init as init process
[    0.174964] Run /bin/sh as init process


BusyBox v1.32.0 (2020-09-24 13:17:53 PDT) hush - the humble shell
Enter 'help' for a list of built-in commands.

After applying this commit all I see is:

Invalid read at addr 0x0, size 8, region '(null)', reason: rejected

It looks like the rng-seed property is causing failures on older kernels.

Alistair

>
> Cc: Alistair Francis <alistair.francis@wdc.com>
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> ---
>  hw/riscv/virt.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index bc424dd2f5..368a723bf6 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -21,6 +21,7 @@
>  #include "qemu/osdep.h"
>  #include "qemu/units.h"
>  #include "qemu/error-report.h"
> +#include "qemu/guest-random.h"
>  #include "qapi/error.h"
>  #include "hw/boards.h"
>  #include "hw/loader.h"
> @@ -998,6 +999,7 @@ static void create_fdt(RISCVVirtState *s, const MemMapEntry *memmap,
>      MachineState *mc = MACHINE(s);
>      uint32_t phandle = 1, irq_mmio_phandle = 1, msi_pcie_phandle = 1;
>      uint32_t irq_pcie_phandle = 1, irq_virtio_phandle = 1;
> +    uint8_t rng_seed[32];
>
>      if (mc->dtb) {
>          mc->fdt = load_device_tree(mc->dtb, &s->fdt_size);
> @@ -1046,6 +1048,10 @@ update_bootargs:
>      if (cmdline && *cmdline) {
>          qemu_fdt_setprop_string(mc->fdt, "/chosen", "bootargs", cmdline);
>      }
> +
> +    /* Pass seed to RNG. */
> +    qemu_guest_getrandom_nofail(rng_seed, sizeof(rng_seed));
> +    qemu_fdt_setprop(mc->fdt, "/chosen", "rng-seed", rng_seed, sizeof(rng_seed));
>  }
>
>  static inline DeviceState *gpex_pcie_init(MemoryRegion *sys_mem,
> --
> 2.35.1
>
>


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] hw/riscv: virt: pass random seed to fdt
  2022-06-29  2:09 ` Alistair Francis
@ 2022-07-05  1:09   ` Jason A. Donenfeld
  2022-07-07  1:04     ` Jason A. Donenfeld
  0 siblings, 1 reply; 16+ messages in thread
From: Jason A. Donenfeld @ 2022-07-05  1:09 UTC (permalink / raw)
  To: Alistair Francis; +Cc: qemu-devel@nongnu.org Developers, Alistair Francis

Hi Alistair,

On Wed, Jun 29, 2022 at 4:09 AM Alistair Francis <alistair23@gmail.com> wrote:
> I have a Linux 5.8 test case that is failing due to this patch.

Before I started fixing things in random.c, there were a lot of early
boot bugs with the RNG in Linux. I backported the fixes for these to
all stable kernels. It's a bummer that risc-v got hit by these bugs,
but I think that's just the way things go unfortunately.

Jason


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] hw/riscv: virt: pass random seed to fdt
  2022-07-05  1:09   ` Jason A. Donenfeld
@ 2022-07-07  1:04     ` Jason A. Donenfeld
  2022-07-08  7:59       ` Alistair Francis
  0 siblings, 1 reply; 16+ messages in thread
From: Jason A. Donenfeld @ 2022-07-07  1:04 UTC (permalink / raw)
  To: Alistair Francis; +Cc: qemu-devel@nongnu.org Developers, Alistair Francis

Hey Alistair,

On Tue, Jul 05, 2022 at 03:09:09AM +0200, Jason A. Donenfeld wrote:
> Hi Alistair,
> 
> On Wed, Jun 29, 2022 at 4:09 AM Alistair Francis <alistair23@gmail.com> wrote:
> > I have a Linux 5.8 test case that is failing due to this patch.
> 
> Before I started fixing things in random.c, there were a lot of early
> boot bugs with the RNG in Linux. I backported the fixes for these to
> all stable kernels. It's a bummer that risc-v got hit by these bugs,
> but I think that's just the way things go unfortunately.
> 
> Jason
> 

By the way, I still can't find this in your github tree. I was hoping we
could get this in for 7.1.

As for your 5.8 issue, I've been trying to reproduce that to understand
more about it, but I'm unable to. I've been trying with
nommu_virt_defconfig using my patch ontop of qemu master. Maybe it's
possible in testing this out you were testing the wrong branch? Anyway,
it'd be nice to get this queued up...

Jason


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] hw/riscv: virt: pass random seed to fdt
  2022-07-07  1:04     ` Jason A. Donenfeld
@ 2022-07-08  7:59       ` Alistair Francis
  2022-07-08  9:56         ` Jason A. Donenfeld
  0 siblings, 1 reply; 16+ messages in thread
From: Alistair Francis @ 2022-07-08  7:59 UTC (permalink / raw)
  To: Jason A. Donenfeld; +Cc: qemu-devel@nongnu.org Developers, Alistair Francis

On Thu, Jul 7, 2022 at 11:04 AM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> Hey Alistair,
>
> On Tue, Jul 05, 2022 at 03:09:09AM +0200, Jason A. Donenfeld wrote:
> > Hi Alistair,
> >
> > On Wed, Jun 29, 2022 at 4:09 AM Alistair Francis <alistair23@gmail.com> wrote:
> > > I have a Linux 5.8 test case that is failing due to this patch.
> >
> > Before I started fixing things in random.c, there were a lot of early
> > boot bugs with the RNG in Linux. I backported the fixes for these to
> > all stable kernels. It's a bummer that risc-v got hit by these bugs,
> > but I think that's just the way things go unfortunately.

Hmm... That's a pain. So there is a bug in older kernels where they
won't boot if we specify this?

Can you point to the fixes?

> >
> > Jason
> >
>
> By the way, I still can't find this in your github tree. I was hoping we
> could get this in for 7.1.

Yeah, it's hard to accept when it will break users. I would rather
avoid someone upgrading to QEMU 7.1 and the kernel failing to boot
with no information.

>
> As for your 5.8 issue, I've been trying to reproduce that to understand
> more about it, but I'm unable to. I've been trying with
> nommu_virt_defconfig using my patch ontop of qemu master. Maybe it's
> possible in testing this out you were testing the wrong branch? Anyway,
> it'd be nice to get this queued up...

Hmm... you can't reproduce it?

Alistair

>
> Jason


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] hw/riscv: virt: pass random seed to fdt
  2022-07-08  7:59       ` Alistair Francis
@ 2022-07-08  9:56         ` Jason A. Donenfeld
  2022-07-11  0:25           ` Alistair Francis
  0 siblings, 1 reply; 16+ messages in thread
From: Jason A. Donenfeld @ 2022-07-08  9:56 UTC (permalink / raw)
  To: Alistair Francis; +Cc: qemu-devel@nongnu.org Developers, Alistair Francis

Hi Alistair,

On 7/8/22, Alistair Francis <alistair23@gmail.com> wrote:

>> > but I think that's just the way things go unfortunately.
>
> Hmm... That's a pain. So there is a bug in older kernels where they
> won't boot if we specify this?
>
> Can you point to the fixes?

Actually, in trying to reproduce this, I don't actually think this is
affected by those old random.c bugs.


>> As for your 5.8 issue, I've been trying to reproduce that to understand
>> more about it, but I'm unable to. I've been trying with
>> nommu_virt_defconfig using my patch ontop of qemu master. Maybe it's
>> possible in testing this out you were testing the wrong branch? Anyway,
>> it'd be nice to get this queued up...
>
> Hmm... you can't reproduce it?

No, I can't, and I'm now no longer convinced that there *is* a bug.
Can you try to repro again and send me detailed reproduction steps?

Thanks,
Jason


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] hw/riscv: virt: pass random seed to fdt
  2022-07-08  9:56         ` Jason A. Donenfeld
@ 2022-07-11  0:25           ` Alistair Francis
  2022-07-11  0:27             ` Jason A. Donenfeld
  0 siblings, 1 reply; 16+ messages in thread
From: Alistair Francis @ 2022-07-11  0:25 UTC (permalink / raw)
  To: Jason A. Donenfeld; +Cc: qemu-devel@nongnu.org Developers, Alistair Francis

On Fri, Jul 8, 2022 at 7:56 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> Hi Alistair,
>
> On 7/8/22, Alistair Francis <alistair23@gmail.com> wrote:
>
> >> > but I think that's just the way things go unfortunately.
> >
> > Hmm... That's a pain. So there is a bug in older kernels where they
> > won't boot if we specify this?
> >
> > Can you point to the fixes?
>
> Actually, in trying to reproduce this, I don't actually think this is
> affected by those old random.c bugs.
>
>
> >> As for your 5.8 issue, I've been trying to reproduce that to understand
> >> more about it, but I'm unable to. I've been trying with
> >> nommu_virt_defconfig using my patch ontop of qemu master. Maybe it's
> >> possible in testing this out you were testing the wrong branch? Anyway,
> >> it'd be nice to get this queued up...
> >
> > Hmm... you can't reproduce it?
>
> No, I can't, and I'm now no longer convinced that there *is* a bug.
> Can you try to repro again and send me detailed reproduction steps?

I just checked again and I can confirm it is this patch that causes
the regression.

This is the command line:

qemu-system-riscv64 \
-machine virt -m 64M \
-cpu rv64,mmu=false \
-serial mon:stdio -serial null -nographic \
-append "root=/dev/vda rw highres=off  console=ttyS0 mem=1G ip=dhcp
earlycon=sbi" \
-device virtio-net-device,netdev=net0,mac=52:54:00:12:34:02 -netdev
user,id=net0 \
-object rng-random,filename=/dev/urandom,id=rng0 -device
virtio-rng-device,rng=rng0 \
-smp 1 -d guest_errors \
-kernel ./images/qemuriscv64/nommu-Image \
-drive id=disk0,file=./images/qemuriscv64/nommu-rootfs.ext2,if=none,format=raw \
-device virtio-blk-device,drive=disk0 -bios none

You can access the images from:
https://nextcloud.alistair23.me/index.php/s/a2zrtbT7DjdTx9t

Alistair

>
> Thanks,
> Jason


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] hw/riscv: virt: pass random seed to fdt
  2022-07-11  0:25           ` Alistair Francis
@ 2022-07-11  0:27             ` Jason A. Donenfeld
  2022-07-11  3:36               ` Alistair Francis
  0 siblings, 1 reply; 16+ messages in thread
From: Jason A. Donenfeld @ 2022-07-11  0:27 UTC (permalink / raw)
  To: Alistair Francis; +Cc: qemu-devel@nongnu.org Developers, Alistair Francis

On 7/11/22, Alistair Francis <alistair23@gmail.com> wrote:
> On Fri, Jul 8, 2022 at 7:56 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>>
>> Hi Alistair,
>>
>> On 7/8/22, Alistair Francis <alistair23@gmail.com> wrote:
>>
>> >> > but I think that's just the way things go unfortunately.
>> >
>> > Hmm... That's a pain. So there is a bug in older kernels where they
>> > won't boot if we specify this?
>> >
>> > Can you point to the fixes?
>>
>> Actually, in trying to reproduce this, I don't actually think this is
>> affected by those old random.c bugs.
>>
>>
>> >> As for your 5.8 issue, I've been trying to reproduce that to
>> >> understand
>> >> more about it, but I'm unable to. I've been trying with
>> >> nommu_virt_defconfig using my patch ontop of qemu master. Maybe it's
>> >> possible in testing this out you were testing the wrong branch?
>> >> Anyway,
>> >> it'd be nice to get this queued up...
>> >
>> > Hmm... you can't reproduce it?
>>
>> No, I can't, and I'm now no longer convinced that there *is* a bug.
>> Can you try to repro again and send me detailed reproduction steps?
>
> I just checked again and I can confirm it is this patch that causes
> the regression.
>
> This is the command line:
>
> qemu-system-riscv64 \
> -machine virt -m 64M \
> -cpu rv64,mmu=false \
> -serial mon:stdio -serial null -nographic \
> -append "root=/dev/vda rw highres=off  console=ttyS0 mem=1G ip=dhcp
> earlycon=sbi" \
> -device virtio-net-device,netdev=net0,mac=52:54:00:12:34:02 -netdev
> user,id=net0 \
> -object rng-random,filename=/dev/urandom,id=rng0 -device
> virtio-rng-device,rng=rng0 \
> -smp 1 -d guest_errors \
> -kernel ./images/qemuriscv64/nommu-Image \
> -drive
> id=disk0,file=./images/qemuriscv64/nommu-rootfs.ext2,if=none,format=raw \
> -device virtio-blk-device,drive=disk0 -bios none
>
> You can access the images from:
> https://nextcloud.alistair23.me/index.php/s/a2zrtbT7DjdTx9t
>

Thanks. Can you send the kernel . config too?

Jason


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] hw/riscv: virt: pass random seed to fdt
  2022-07-11  0:27             ` Jason A. Donenfeld
@ 2022-07-11  3:36               ` Alistair Francis
  2022-07-11 16:45                 ` Jason A. Donenfeld
  0 siblings, 1 reply; 16+ messages in thread
From: Alistair Francis @ 2022-07-11  3:36 UTC (permalink / raw)
  To: Jason A. Donenfeld; +Cc: qemu-devel@nongnu.org Developers, Alistair Francis

On Mon, Jul 11, 2022 at 10:28 AM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> On 7/11/22, Alistair Francis <alistair23@gmail.com> wrote:
> > On Fri, Jul 8, 2022 at 7:56 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> >>
> >> Hi Alistair,
> >>
> >> On 7/8/22, Alistair Francis <alistair23@gmail.com> wrote:
> >>
> >> >> > but I think that's just the way things go unfortunately.
> >> >
> >> > Hmm... That's a pain. So there is a bug in older kernels where they
> >> > won't boot if we specify this?
> >> >
> >> > Can you point to the fixes?
> >>
> >> Actually, in trying to reproduce this, I don't actually think this is
> >> affected by those old random.c bugs.
> >>
> >>
> >> >> As for your 5.8 issue, I've been trying to reproduce that to
> >> >> understand
> >> >> more about it, but I'm unable to. I've been trying with
> >> >> nommu_virt_defconfig using my patch ontop of qemu master. Maybe it's
> >> >> possible in testing this out you were testing the wrong branch?
> >> >> Anyway,
> >> >> it'd be nice to get this queued up...
> >> >
> >> > Hmm... you can't reproduce it?
> >>
> >> No, I can't, and I'm now no longer convinced that there *is* a bug.
> >> Can you try to repro again and send me detailed reproduction steps?
> >
> > I just checked again and I can confirm it is this patch that causes
> > the regression.
> >
> > This is the command line:
> >
> > qemu-system-riscv64 \
> > -machine virt -m 64M \
> > -cpu rv64,mmu=false \
> > -serial mon:stdio -serial null -nographic \
> > -append "root=/dev/vda rw highres=off  console=ttyS0 mem=1G ip=dhcp
> > earlycon=sbi" \
> > -device virtio-net-device,netdev=net0,mac=52:54:00:12:34:02 -netdev
> > user,id=net0 \
> > -object rng-random,filename=/dev/urandom,id=rng0 -device
> > virtio-rng-device,rng=rng0 \
> > -smp 1 -d guest_errors \
> > -kernel ./images/qemuriscv64/nommu-Image \
> > -drive
> > id=disk0,file=./images/qemuriscv64/nommu-rootfs.ext2,if=none,format=raw \
> > -device virtio-blk-device,drive=disk0 -bios none
> >
> > You can access the images from:
> > https://nextcloud.alistair23.me/index.php/s/a2zrtbT7DjdTx9t
> >
>
> Thanks. Can you send the kernel . config too?

Unfortunately I don't have it, it should just be the 5.8 no MMU defconfig though

Alistair

>
> Jason


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] hw/riscv: virt: pass random seed to fdt
  2022-07-11  3:36               ` Alistair Francis
@ 2022-07-11 16:45                 ` Jason A. Donenfeld
  2022-07-13 17:29                   ` Jason A. Donenfeld
  0 siblings, 1 reply; 16+ messages in thread
From: Jason A. Donenfeld @ 2022-07-11 16:45 UTC (permalink / raw)
  To: Alistair Francis; +Cc: qemu-devel@nongnu.org Developers, Alistair Francis

Hi Alistair,

On Mon, Jul 11, 2022 at 01:36:28PM +1000, Alistair Francis wrote:
> On Mon, Jul 11, 2022 at 10:28 AM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> >
> > On 7/11/22, Alistair Francis <alistair23@gmail.com> wrote:
> > > On Fri, Jul 8, 2022 at 7:56 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> > >>
> > >> Hi Alistair,
> > >>
> > >> On 7/8/22, Alistair Francis <alistair23@gmail.com> wrote:
> > >>
> > >> >> > but I think that's just the way things go unfortunately.
> > >> >
> > >> > Hmm... That's a pain. So there is a bug in older kernels where they
> > >> > won't boot if we specify this?
> > >> >
> > >> > Can you point to the fixes?
> > >>
> > >> Actually, in trying to reproduce this, I don't actually think this is
> > >> affected by those old random.c bugs.
> > >>
> > >>
> > >> >> As for your 5.8 issue, I've been trying to reproduce that to
> > >> >> understand
> > >> >> more about it, but I'm unable to. I've been trying with
> > >> >> nommu_virt_defconfig using my patch ontop of qemu master. Maybe it's
> > >> >> possible in testing this out you were testing the wrong branch?
> > >> >> Anyway,
> > >> >> it'd be nice to get this queued up...
> > >> >
> > >> > Hmm... you can't reproduce it?
> > >>
> > >> No, I can't, and I'm now no longer convinced that there *is* a bug.
> > >> Can you try to repro again and send me detailed reproduction steps?
> > >
> > > I just checked again and I can confirm it is this patch that causes
> > > the regression.
> > >
> > > This is the command line:
> > >
> > > qemu-system-riscv64 \
> > > -machine virt -m 64M \
> > > -cpu rv64,mmu=false \
> > > -serial mon:stdio -serial null -nographic \
> > > -append "root=/dev/vda rw highres=off  console=ttyS0 mem=1G ip=dhcp
> > > earlycon=sbi" \
> > > -device virtio-net-device,netdev=net0,mac=52:54:00:12:34:02 -netdev
> > > user,id=net0 \
> > > -object rng-random,filename=/dev/urandom,id=rng0 -device
> > > virtio-rng-device,rng=rng0 \
> > > -smp 1 -d guest_errors \
> > > -kernel ./images/qemuriscv64/nommu-Image \
> > > -drive
> > > id=disk0,file=./images/qemuriscv64/nommu-rootfs.ext2,if=none,format=raw \
> > > -device virtio-blk-device,drive=disk0 -bios none
> > >
> > > You can access the images from:
> > > https://nextcloud.alistair23.me/index.php/s/a2zrtbT7DjdTx9t
> > >
> >
> > Thanks. Can you send the kernel . config too?
> 
> Unfortunately I don't have it, it should just be the 5.8 no MMU defconfig though

I've reproduced the problem and determined the root cause. This is a
generic issue with the mmio get_cycles() implementation before 5.9 on
no-MMU configs, which was fixed during the 5.9 cycle. I don't believe
that this is the only thing affected on that .0 kernel, where fixes were
ostensibly backported. Given the relative age of risc-v, the fact that
5.8.0 was broken anyway, and that likely nobody is using this kernel in
that configuration without applying updates, I'm pretty sure my patch is
safe to apply. I'd recommend updating the broken kernel in your CI.

Meanwhile, the rng-seed field is part of the DT spec. Holding back the
(virtual) hardware just because some random dot-zero non-LTS release had
a quickly fixed bug seems ridiculous, and the way in which progress gets
held up, hacks accumulate, and generally nothing good gets done. It will
only hamper security, functionality, and boot speed, while helping no
real practical case that can't be fixed in a better way.

So I believe you should apply the rng-seed commit so that the RISC-V
machine honors that DT field.

Regards,
Jason


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] hw/riscv: virt: pass random seed to fdt
  2022-07-11 16:45                 ` Jason A. Donenfeld
@ 2022-07-13 17:29                   ` Jason A. Donenfeld
  2022-07-18 22:39                     ` Alistair Francis
  0 siblings, 1 reply; 16+ messages in thread
From: Jason A. Donenfeld @ 2022-07-13 17:29 UTC (permalink / raw)
  To: Alistair Francis; +Cc: qemu-devel@nongnu.org Developers, Alistair Francis

Hi again,

On Mon, Jul 11, 2022 at 06:45:42PM +0200, Jason A. Donenfeld wrote:
> I've reproduced the problem and determined the root cause. This is a
> generic issue with the mmio get_cycles() implementation before 5.9 on
> no-MMU configs, which was fixed during the 5.9 cycle. I don't believe
> that this is the only thing affected on that .0 kernel, where fixes were
> ostensibly backported. Given the relative age of risc-v, the fact that
> 5.8.0 was broken anyway, and that likely nobody is using this kernel in
> that configuration without applying updates, I'm pretty sure my patch is
> safe to apply. I'd recommend updating the broken kernel in your CI.
> 
> Meanwhile, the rng-seed field is part of the DT spec. Holding back the
> (virtual) hardware just because some random dot-zero non-LTS release had
> a quickly fixed bug seems ridiculous, and the way in which progress gets
> held up, hacks accumulate, and generally nothing good gets done. It will
> only hamper security, functionality, and boot speed, while helping no
> real practical case that can't be fixed in a better way.
> 
> So I believe you should apply the rng-seed commit so that the RISC-V
> machine honors that DT field.
> 
> Regards,
> Jason
> 

Just following up on this... Hoping we can get this into a tree soon.

Thanks,
Jason


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] hw/riscv: virt: pass random seed to fdt
  2022-07-13 17:29                   ` Jason A. Donenfeld
@ 2022-07-18 22:39                     ` Alistair Francis
  0 siblings, 0 replies; 16+ messages in thread
From: Alistair Francis @ 2022-07-18 22:39 UTC (permalink / raw)
  To: Jason A. Donenfeld; +Cc: qemu-devel@nongnu.org Developers, Alistair Francis

On Thu, Jul 14, 2022 at 3:29 AM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> Hi again,
>
> On Mon, Jul 11, 2022 at 06:45:42PM +0200, Jason A. Donenfeld wrote:
> > I've reproduced the problem and determined the root cause. This is a
> > generic issue with the mmio get_cycles() implementation before 5.9 on
> > no-MMU configs, which was fixed during the 5.9 cycle. I don't believe
> > that this is the only thing affected on that .0 kernel, where fixes were
> > ostensibly backported. Given the relative age of risc-v, the fact that
> > 5.8.0 was broken anyway, and that likely nobody is using this kernel in
> > that configuration without applying updates, I'm pretty sure my patch is
> > safe to apply. I'd recommend updating the broken kernel in your CI.
> >
> > Meanwhile, the rng-seed field is part of the DT spec. Holding back the
> > (virtual) hardware just because some random dot-zero non-LTS release had
> > a quickly fixed bug seems ridiculous, and the way in which progress gets
> > held up, hacks accumulate, and generally nothing good gets done. It will
> > only hamper security, functionality, and boot speed, while helping no
> > real practical case that can't be fixed in a better way.
> >
> > So I believe you should apply the rng-seed commit so that the RISC-V
> > machine honors that DT field.
> >
> > Regards,
> > Jason
> >
>
> Just following up on this... Hoping we can get this into a tree soon.

Yep! Sorry, I have been off sick for the last week.

I just updated my test images to a newer kernel, which means this
passes my tests

Thanks!

Applied to riscv-to-apply.next

Alistair

>
> Thanks,
> Jason


^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2022-07-18 22:41 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-13 11:58 [PATCH] hw/riscv: virt: pass random seed to fdt Jason A. Donenfeld
2022-06-15  4:05 ` Bin Meng
2022-06-16  2:32   ` Alistair Francis
2022-06-16 10:01     ` Jason A. Donenfeld
2022-06-16 12:17       ` Alistair Francis
2022-06-29  2:09 ` Alistair Francis
2022-07-05  1:09   ` Jason A. Donenfeld
2022-07-07  1:04     ` Jason A. Donenfeld
2022-07-08  7:59       ` Alistair Francis
2022-07-08  9:56         ` Jason A. Donenfeld
2022-07-11  0:25           ` Alistair Francis
2022-07-11  0:27             ` Jason A. Donenfeld
2022-07-11  3:36               ` Alistair Francis
2022-07-11 16:45                 ` Jason A. Donenfeld
2022-07-13 17:29                   ` Jason A. Donenfeld
2022-07-18 22:39                     ` Alistair Francis

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.