| Message ID | 20240912125132.268802-7-gaosong@loongson.cn (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [PULL,1/7] target/loongarch: Add compatible support about VM reboot | expand |
Hi Song / Jason, We're seeing non-deterministic hangs in our functional test suite 'tests/functional/test_loongarch64_virt.py' and my attempt at git bisect is blaming this commit. With this applied, perhaps 1 time in 10, the test case hangs, with zero serial port output from EDK2 emitted https://gitlab.com/qemu-project/qemu/-/issues/2686 I'm not seeing an obvious flaw in this commit's code, so I'm wondering if it has accidentally tickled some other subtle pre-existing bug elsewhere. Some general problem with FDT handling on loongarch64 perhaps ? On Thu, Sep 12, 2024 at 08:51:31PM +0800, Song Gao wrote: > From: "Jason A. Donenfeld" <Jason@zx2c4.com> > > 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 the same procedure that's done in b91b6b5a2c ("hw/microblaze: > pass random seed to fdt"), e4b4f0b71c ("hw/riscv: virt: pass random seed > to fdt"), c6fe3e6b4c ("hw/openrisc: virt: pass random seed to fdt"), > 67f7e426e5 ("hw/i386: pass RNG seed via setup_data entry"), c287941a4d > ("hw/rx: pass random seed to fdt"), 5e19cc68fb ("hw/mips: boston: pass > random seed to fdt"), 6b23a67916 ("hw/nios2: virt: pass random seed to fdt") > c4b075318e ("hw/ppc: pass random seed to fdt"), and 5242876f37 > ("hw/arm/virt: dt: add rng-seed property"). > > These earlier commits later were amended to rerandomize the RNG seed on > snapshot load, but the LoongArch code somehow already does that, despite > not having this patch here, presumably due to some lucky copy and > pasting. > > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> > Reviewed-by: Song Gao <gaosong@loongson.cn> > Message-Id: <20240905153316.2038769-1-Jason@zx2c4.com> > Signed-off-by: Song Gao <gaosong@loongson.cn> > --- > hw/loongarch/virt.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c > index 76dd29a391..81b1f9486f 100644 > --- a/hw/loongarch/virt.c > +++ b/hw/loongarch/virt.c > @@ -48,6 +48,7 @@ > #include "hw/block/flash.h" > #include "hw/virtio/virtio-iommu.h" > #include "qemu/error-report.h" > +#include "qemu/guest-random.h" > > static bool virt_is_veiointc_enabled(LoongArchVirtMachineState *lvms) > { > @@ -303,6 +304,7 @@ static void fdt_add_uart_node(LoongArchVirtMachineState *lvms, > static void create_fdt(LoongArchVirtMachineState *lvms) > { > MachineState *ms = MACHINE(lvms); > + uint8_t rng_seed[32]; > > ms->fdt = create_device_tree(&lvms->fdt_size); > if (!ms->fdt) { > @@ -316,6 +318,10 @@ static void create_fdt(LoongArchVirtMachineState *lvms) > qemu_fdt_setprop_cell(ms->fdt, "/", "#address-cells", 0x2); > qemu_fdt_setprop_cell(ms->fdt, "/", "#size-cells", 0x2); > qemu_fdt_add_subnode(ms->fdt, "/chosen"); > + > + /* Pass seed to RNG */ > + qemu_guest_getrandom_nofail(rng_seed, sizeof(rng_seed)); > + qemu_fdt_setprop(ms->fdt, "/chosen", "rng-seed", rng_seed, sizeof(rng_seed)); > } > > static void fdt_add_cpu_nodes(const LoongArchVirtMachineState *lvms) > -- > 2.34.1 > > With regards, Daniel
Hi Daniel, Thanks for reporting the problem. I can reproduce this problem with my local environment, we will investigate this issue ASAP. Regards Bibo Mao On 2024/11/19 上午3:57, Daniel P. Berrangé wrote: > Hi Song / Jason, > > We're seeing non-deterministic hangs in our functional test > suite 'tests/functional/test_loongarch64_virt.py' and my > attempt at git bisect is blaming this commit. > > With this applied, perhaps 1 time in 10, the test case hangs, > with zero serial port output from EDK2 emitted > > https://gitlab.com/qemu-project/qemu/-/issues/2686 > > I'm not seeing an obvious flaw in this commit's code, so I'm > wondering if it has accidentally tickled some other subtle > pre-existing bug elsewhere. Some general problem with FDT > handling on loongarch64 perhaps ? > > On Thu, Sep 12, 2024 at 08:51:31PM +0800, Song Gao wrote: >> From: "Jason A. Donenfeld" <Jason@zx2c4.com> >> >> 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 the same procedure that's done in b91b6b5a2c ("hw/microblaze: >> pass random seed to fdt"), e4b4f0b71c ("hw/riscv: virt: pass random seed >> to fdt"), c6fe3e6b4c ("hw/openrisc: virt: pass random seed to fdt"), >> 67f7e426e5 ("hw/i386: pass RNG seed via setup_data entry"), c287941a4d >> ("hw/rx: pass random seed to fdt"), 5e19cc68fb ("hw/mips: boston: pass >> random seed to fdt"), 6b23a67916 ("hw/nios2: virt: pass random seed to fdt") >> c4b075318e ("hw/ppc: pass random seed to fdt"), and 5242876f37 >> ("hw/arm/virt: dt: add rng-seed property"). >> >> These earlier commits later were amended to rerandomize the RNG seed on >> snapshot load, but the LoongArch code somehow already does that, despite >> not having this patch here, presumably due to some lucky copy and >> pasting. >> >> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> >> Reviewed-by: Song Gao <gaosong@loongson.cn> >> Message-Id: <20240905153316.2038769-1-Jason@zx2c4.com> >> Signed-off-by: Song Gao <gaosong@loongson.cn> >> --- >> hw/loongarch/virt.c | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c >> index 76dd29a391..81b1f9486f 100644 >> --- a/hw/loongarch/virt.c >> +++ b/hw/loongarch/virt.c >> @@ -48,6 +48,7 @@ >> #include "hw/block/flash.h" >> #include "hw/virtio/virtio-iommu.h" >> #include "qemu/error-report.h" >> +#include "qemu/guest-random.h" >> >> static bool virt_is_veiointc_enabled(LoongArchVirtMachineState *lvms) >> { >> @@ -303,6 +304,7 @@ static void fdt_add_uart_node(LoongArchVirtMachineState *lvms, >> static void create_fdt(LoongArchVirtMachineState *lvms) >> { >> MachineState *ms = MACHINE(lvms); >> + uint8_t rng_seed[32]; >> >> ms->fdt = create_device_tree(&lvms->fdt_size); >> if (!ms->fdt) { >> @@ -316,6 +318,10 @@ static void create_fdt(LoongArchVirtMachineState *lvms) >> qemu_fdt_setprop_cell(ms->fdt, "/", "#address-cells", 0x2); >> qemu_fdt_setprop_cell(ms->fdt, "/", "#size-cells", 0x2); >> qemu_fdt_add_subnode(ms->fdt, "/chosen"); >> + >> + /* Pass seed to RNG */ >> + qemu_guest_getrandom_nofail(rng_seed, sizeof(rng_seed)); >> + qemu_fdt_setprop(ms->fdt, "/chosen", "rng-seed", rng_seed, sizeof(rng_seed)); >> } >> >> static void fdt_add_cpu_nodes(const LoongArchVirtMachineState *lvms) >> -- >> 2.34.1 >> >> > > With regards, > Daniel >
diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c index 76dd29a391..81b1f9486f 100644 --- a/hw/loongarch/virt.c +++ b/hw/loongarch/virt.c @@ -48,6 +48,7 @@ #include "hw/block/flash.h" #include "hw/virtio/virtio-iommu.h" #include "qemu/error-report.h" +#include "qemu/guest-random.h" static bool virt_is_veiointc_enabled(LoongArchVirtMachineState *lvms) { @@ -303,6 +304,7 @@ static void fdt_add_uart_node(LoongArchVirtMachineState *lvms, static void create_fdt(LoongArchVirtMachineState *lvms) { MachineState *ms = MACHINE(lvms); + uint8_t rng_seed[32]; ms->fdt = create_device_tree(&lvms->fdt_size); if (!ms->fdt) { @@ -316,6 +318,10 @@ static void create_fdt(LoongArchVirtMachineState *lvms) qemu_fdt_setprop_cell(ms->fdt, "/", "#address-cells", 0x2); qemu_fdt_setprop_cell(ms->fdt, "/", "#size-cells", 0x2); qemu_fdt_add_subnode(ms->fdt, "/chosen"); + + /* Pass seed to RNG */ + qemu_guest_getrandom_nofail(rng_seed, sizeof(rng_seed)); + qemu_fdt_setprop(ms->fdt, "/chosen", "rng-seed", rng_seed, sizeof(rng_seed)); } static void fdt_add_cpu_nodes(const LoongArchVirtMachineState *lvms)